-
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
Update single fetch logic when clientLoaders are present #9073
Conversation
🦋 Changeset detectedLatest commit: be61761 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
// When a route has a client loader, we make it's own call for just | ||
// it's server loader data | ||
let url = stripIndexParam(singleFetchUrl(request.url)); | ||
url.searchParams.set("_routes", routeId); | ||
let { data } = await fetchAndDecode(url); | ||
results = data as SingleFetchResults; |
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.
clientLoader
calls through to serverLoader
make a call for their individual route
if (!singleFetchPromise) { | ||
let url = addRevalidationParam( | ||
manifest, | ||
routeModules, | ||
matches.map((m) => m.route), | ||
matches.filter((m) => m.shouldLoad).map((m) => m.route), | ||
stripIndexParam(singleFetchUrl(request.url)) | ||
); | ||
singleFetchPromise = fetchAndDecode(url).then( | ||
({ data }) => data as SingleFetchResults | ||
); | ||
} | ||
results = await singleFetchPromise; |
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.
Everyone else (non-clientLoader
routes) latches onto the same promise
manifest.routes[r.id]?.hasClientLoader | ||
); | ||
if (!needsParam) { | ||
return url; |
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 don't bother adding a param if there are no shouldRevalidate
's and no clientLoader
's
let loadIds = genRouteIds( | ||
loadRoutes | ||
.filter((r) => !manifest.routes[r.id]?.hasClientLoader) | ||
.map((r) => r.id) | ||
); |
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.
Load the routes requested that don't have client loaders
If I understand correctly, by adding a clientLoader you're essentially opting out of Single Fetch? So now every |
Yeah only for that route branch. Otherwise we nullify a primary benefit of You still get a multi-route single fetch call for any routes that do not have a client loader, so if you route to
|
Change the logic for
serverLoader
calls fromclientLoader
's in single fetch mode so we're not over-fetching on the server if aclientLoader
never callsserverLoader
.Todo: