-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support prerender in Netlify redirects #5904
Conversation
🦋 Changeset detectedLatest commit: 1fdc937 The changes in this PR will be included in the next version bump. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see what needs to be fixed now. I tested this locally, I wonder if the functions/fixtures/404/dist/_redirects
is correct now though. It is:
/* /.netlify/functions/entry 404
/ /.netlify/functions/entry 200
/404 /.netlify/functions/entry 200
where the /*
is the first and I think that causes a 404 always? Before this PR it's:
/ /.netlify/functions/entry 200
/404 /.netlify/functions/entry 200
/* /.netlify/functions/entry 404
I think we need to sort to like before for this case.
Ah yeah, you're right. I think I was actually wrong about needing to sort dynamic routes first. What you really want is to sort any exact route first. It's the wildcard/spread routes that need to come last. I think I'll add a |
expect(redir).to.include('/pets/:cat /pets/:cat/index.html 200'); | ||
expect(redir).to.include('/pets/:dog /pets/:dog/index.html 200'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will these conflict? I expect that they might and only /pets/:cat
would be matched.
Since all of these are generating actual files we might need to track any files that the dynamic route has generated, then add redirects for the exact route?
expect(redir).to.include('/pets/cat1 /pets/cat1/index.html 200');
expect(redir).to.include('/pets/cat2 /pets/cat2/index.html 200');
expect(redir).to.include('/pets/cat3 /pets/cat3/index.html 200');
expect(redir).to.include('/pets/dog1 /pets/dog1/index.html 200');
expect(redir).to.include('/pets/dog2 /pets/dog2/index.html 200');
expect(redir).to.include('/pets/dog3 /pets/dog3/index.html 200');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They do conflict in an SSR context but in SSG since they are pre-generated they are just redundant. Meaning Netlify is going to use the first one and ignore the second; but that doesn't matter because it will go to the right file anyways.
The user should probably combine these into 1 route, but we don't require that at the moment so this is a valid (if odd) way to split up different "types" of data in SSG.
Ok, I think I got it now @bluwy here: 00224f3 The changes are:
|
Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
* Support prerender in Netlify redirects * Updated sorting algorithm * Update packages/integrations/netlify/src/shared.ts Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com> Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
Changes
Testing
Docs
N/A, bug fix.