Conversation
🦋 Changeset detectedLatest commit: 517ffa3 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 |
750c04e to
72dfaf7
Compare
ematipico
commented
Nov 16, 2023
| `[${staticPaths.length} ${label}]` | ||
| )}` | ||
| ); | ||
| for (const route of eachRouteInRouteData(pageData)) { |
Member
Author
There was a problem hiding this comment.
Now we loop over all the routes: the main ones and the fallback routes associated to it
ematipico
commented
Nov 16, 2023
| const matchedRoute = matchRoute(staticPath, opts.manifest); | ||
| return matchedRoute === route; | ||
| }); | ||
| paths.push( |
Member
Author
There was a problem hiding this comment.
We chain all the generated routes all together
ematipico
commented
Nov 16, 2023
| } | ||
|
|
||
| pipeline.getEnvironment().logger.debug('build', `Generating: ${pathname}`); | ||
| for (const route of eachRouteInRouteData(pageData)) { |
Member
Author
There was a problem hiding this comment.
The only relevant change is that the whole logic is inside this loop now, and instead of doing pageData.route, we pass the route from this loop
matthewp
approved these changes
Nov 16, 2023
bluwy
approved these changes
Nov 17, 2023
matthewp
reviewed
Nov 17, 2023
matthewp
reviewed
Nov 17, 2023
matthewp
reviewed
Nov 17, 2023
bluwy
reviewed
Nov 17, 2023
Merged
FredKSchott
pushed a commit
that referenced
this pull request
Nov 18, 2023
natemoo-re
pushed a commit
that referenced
this pull request
Nov 22, 2023
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
This PR closes #9108
The was a big flaw in the logic of the fallback, so I went back, and restored how the routes were stored before the introduction of the fallback logic.
The new change involves the introduction of
fallbackRoutesas part of theRouteDataobject, very very similar to how we do it forredirectRoutes.Although, now the generation of the pages is different, and each
RouteDatacould create more than one page (itself +fallbackRoutes), hence I create a generator function calledeachRouteInRouteData, which returns all the routes we need to create.EDIT:
In SSR, the middleware already has the logic for doing the redirect, although we need to tell it somehow. So I updated the logic of
app/index.ts.The
matchfunction now considers' fallbackRoutes' when matching a route. However, when returning aResponse, we set a status code 302 because we want the fallback to apply.Example:
ittoen/it/blog/articleroute/en/blog/article, because it contains a fallback route that has a pathname that matches/it/blog/article302status code. The middleware catches the status code and returns a redirect to/en/blog/articleTesting
I added a new case where we check the presence of a hoisted script.
The other rests should still pass.
Docs