-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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 lazy route discovery (fog of war) #11626
Conversation
🦋 Changeset detectedLatest commit: ce23383 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
* @param routeId The parent route id | ||
* @param children The additional children routes | ||
*/ | ||
patchRoutes(routeId: string | null, children: AgnosticRouteObject[]): void; |
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.
Used by Remix to eagerly/imperatively patch routes in prior to navigations
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.
Why is this considered private? Wouldn't this be an important part of the public API for a feature like this?
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.
Only for advanced pre-fetching use cases - not the common use case where you'll use patch
from patchRoutesOnMiss
. This is more so just to stay in line with the current public API of router
which is that it's all marked private and we prefer you use the React Router APIs in the vast majority of cases. In v7 we won't have a standalone @remix-run/router
package but we will want to start documenting the true "public" APIs of the router
/** | ||
* Route matches which may have been updated from fog of war discovery | ||
*/ | ||
matches?: RouterState["matches"]; |
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.
We can now return updated matches from fog of war during loader/action execution flows
let fogOfWar = checkFogOfWar(matches, routesToUse, location.pathname); | ||
if (fogOfWar.active && fogOfWar.matches) { | ||
matches = fogOfWar.matches; | ||
} |
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.
Detect if this is a fog of war scenario and we should bypass a synchronous 404 and go into our loading/submitting state to perform discovery
let discoverResult = await discoverRoutes( | ||
matches, | ||
location.pathname, | ||
request.signal | ||
); |
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.
Try to discover a matching route during the submitting
phase of an action
], | ||
}; | ||
} else { | ||
matches = discoverResult.matches; |
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.
Use the lazily discovered matches for the action execution
let discoverResult = await discoverRoutes( | ||
matches, | ||
location.pathname, | ||
request.signal | ||
); |
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.
Perform lazy route discovery during the loading
phase of a GET navigation
requestMatches, | ||
path, | ||
fetchRequest.signal | ||
); |
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.
Fog of war on a fetcher action submission
let fogMatches = matchRoutesImpl<AgnosticDataRouteObject>( | ||
routesToUse, | ||
pathname, | ||
basename, | ||
true | ||
); |
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.
This true
matches as far down as we can
let leafRoute = matches[matches.length - 1].route; | ||
if (leafRoute.path === "*") { | ||
// If we matched a splat, it might only be because we haven't yet fetched | ||
// the children that would match with a higher score, so let's fetch | ||
// around and find out | ||
let partialMatches = matchRoutesImpl<AgnosticDataRouteObject>( | ||
routesToUse, | ||
pathname, | ||
basename, | ||
true | ||
); | ||
return { active: true, matches: partialMatches }; |
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.
Splats are tricky because they might match as a catch-all but there might be a better match if we run through patchRoutesOnMiss
- so we treat them as a miss and if we match then splat again after a patchRouteOnMiss
execution then we'll use it.
): Promise<DiscoverRoutesResult> { | ||
let partialMatches: AgnosticDataRouteMatch[] | null = matches; | ||
let route = partialMatches[partialMatches.length - 1].route; | ||
while (true) { |
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.
Loop through potentially many patchRoutesOnMiss
calls until we officially 404 or we find a match
packages/router/router.ts
Outdated
// We are no longer partially matching anything new so this was either a | ||
// legit splat match above, or it's a 404 | ||
if (matchedSplat) { | ||
return { type: "success", matches: newMatches }; |
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.
If we gave it another run for a splat match and we end up matching the same exact matches, then the splat is actually the correct match and we can use it
Co-authored-by: Mark Dalgleish <[email protected]>
🤖 Hello there, We just published version Thanks! |
🤖 Hello there, We just published version Thanks! |
Original RFC: #11113
We changed the API to a more granular API that could patch down at the individual route level since we found that for large enough apps, the
children()
API would require potentially patching way too many routes for a single link click.Closes the RR half of remix-run/remix#8423