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 |
packages/adapter-vercel/index.js
Outdated
| @@ -446,7 +446,7 @@ function static_vercel_config(builder, config, dir) { | |||
|
|
|||
| for (const [src, redirect] of builder.prerendered.redirects) { | |||
| prerendered_redirects.push({ | |||
| 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
packages/adapter-vercel/index.js
Outdated
| 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 😆
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