fix: disregard presence/absence of trailing slash in prerendered redirect - #12966
Conversation
🦋 Changeset detectedLatest commit: ad68def 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 |
| src, | ||
| src: src.replace(/\/?$/, '/?'), | ||
| headers: { | ||
| Location: redirect.location |
There was a problem hiding this comment.
Is there any chance that the redirect location is the same except the trailing slash, in which case this would create an infinite loop? Maybe we should double-check that before applying the /? replacement
There was a problem hiding this comment.
Super edge case, since you can't create a /foo route alongside a /foo/ route (best you could do is /foo alongside /[blah]/, but one would match over the other). But I guess you could do something wacky with a handle hook or a custom server, and it does indeed create an infinite loop. Pushed some code to handle that case
| for (const [src, redirect] of builder.prerendered.redirects) { | ||
| for (let [src, redirect] of builder.prerendered.redirects) { | ||
| if (src.replace(/\/$/, '') !== redirect.location.replace(/\/$/, '')) { | ||
| // handle the extreme edge case of a `/foo` -> `/foo/` redirect, |
There was a problem hiding this comment.
I'm on mobile, so maybe I'm just misreading, but I feel like this comment should apply to the if statement rather than this line
This line could probably use a different comment about disregarding the presence/absence of trailing slash in prerendered redirect
There was a problem hiding this comment.
yeah the position of the comment is tricky because even if it's outside the if it looks like it's describing what happens inside rather than when you don't get there
fixed slightly unconventionally: inverted the condition and made the antecedent a no-op, just the comment
There was a problem hiding this comment.
lol. that works
I probably would have done it by changing the wording to something like "only apply if we're not hitting the extreme edge case of..."
but I guess I'm happy with this too. maybe the bundler will make them the same anyway 😆
…ic (#16497) adapter-static's `static_vercel_config` is a marked copy of the adapter-vercel function (both carry a "This function is duplicated in..." comment), but two fixes only landed on the adapter-vercel side. This ports #8766 (redirect the non-trailing-slash counterpart of every prerendered page) and #12966 (match prerendered redirect sources regardless of trailing slash), so a static site deployed on Vercel gets the same redirect behavior as adapter-vercel. --- ### Please don't delete this checklist! Before submitting the PR, please make sure you do the following: - [ ] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] This message body should clearly illustrate what problems it solves. - [ ] Ideally, include a test that fails without this PR but passes with it. ### Tests - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check` ### Changesets - [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. Changesets that add features should be `minor` and those that fix bugs should be `patch`. Please prefix changeset messages with `feat:`, `fix:`, or `chore:`. ### Edits - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.
closes #12869. With this change, if the prerenderer encounters a redirect when rendering
/foo, the resulting entry invercel.jsonwill have asrcof/foo/?rather than/fooor/foo/.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits