fix: preserve trailing slash when error boundary truncates branch#15265
fix: preserve trailing slash when error boundary truncates branch#15265FrankFMY wants to merge 2 commits intosveltejs:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 4d16a90 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The CI failures will need to be fixed I'm curious, how are you picking things to work on? We're trying to get ready to start working on SvelteKit 3, and have some big plans for it. We don't necessarily want to get bogged down reviewing smaller issues if they're not things people are frequently running into or are causing significant difficulty for users I'm also curious, is there a particular model you're using? I've still been testing a few different ones out myself |
When a page's load function throws an error and the branch is truncated to the nearest error boundary, the page-level trailingSlash config is lost. This causes the URL to change (e.g. /test/ → /test). Default to 'ignore' instead of 'never' when an error is present, so the URL pathname stays unchanged. If a layout above the error boundary has its own trailingSlash config, it still overrides the default. Fixes sveltejs#13516
98f66e4 to
9d94d89
Compare
|
Thanks for the feedback! I'm using Claude Code CLI with Claude 4.6 Opus — working on open-source contributions in my spare time so my limits don't go to waste, and I get to help projects I genuinely care about. For picking issues — I tend to look for bugs that are clear correctness issues with a small fix surface, so the review burden stays minimal. This one seemed like a good fit: it's a real bug affecting anyone using trailingSlash with error boundaries, the fix is a single line, and it's been open for a while. But I totally understand the priority shift towards SvelteKit 3 — happy to focus on issues that are more impactful or aligned with that direction if you have suggestions. Also, I just genuinely enjoy contributing back to projects I use daily — I have plenty of energy outside of work and got tired of writing code just for myself. Figured I'd put that energy into something the community actually benefits from. Using AI tooling also helps me write clearer commit messages and descriptions, which hopefully makes reviewing easier on your end. |
|
Closing in favour of #15358 which takes into account the trailing slash option set by the user |
Summary
trailingSlash = 'always'is set at the page level and the page'sloadfunction throws an error, the client-side error handling truncates the branch to the nearest error boundary — losing the page node'strailingSlashconfig and causing the URL to change (e.g./test/→/test)'ignore'instead of'never'when an error is present inget_navigation_result_from_branch, so the URL pathname stays unchangedtrailingSlashconfig, it still overrides the default via the branch iterationFixes #13516
Test plan
routing/trailing-slash/error/always-error/withtrailingSlash = 'always'and a load function that throwserror(500)