-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add future flag for Single Fetch #8773
Conversation
… to existing loaders
🦋 Changeset detectedLatest commit: 8d4c012 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 |
packages/remix-react/browser.tsx
Outdated
v7_skipActionErrorRevalidation: | ||
window.__remixContext.future.unstable_singleFetch === 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.
Single fetch opts into this behavior automatically
if (!route.hasLoader) return null; | ||
return fetchServerHandler(request, route); |
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.
Instead of always hitting the server here, if single fetch is enabled we will have passed the singleFetch
method down from dataStrategy
so use that to ensure we make the right HTTP call to the server
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.
By proxying this all the way though and branching here as deep as possible - we avoid having to re-implement any of the logic around critical/lazy routes, revalidation, client data etc. in data
strategy and get to leverage all of the existing battle-tested flows.
packages/remix-react/single-fetch.ts
Outdated
singleFetch = async (routeId) => { | ||
let url = singleFetchUrl(request.url); | ||
let init = await createRequestInit(request); | ||
let result = await fetchAndDecode(url, init); | ||
return unwrapSingleFetchResult(result as SingleFetchResult, routeId); | ||
}; |
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.
Action implementation.
Below is the more complex loader implementation which lets all loaders latch onto a single promise for the call.
packages/remix-react/single-fetch.ts
Outdated
// `serverLoader` and we never read the response of that route from the | ||
// single fetch call, and thus executing that `loader` on the server was | ||
// unnecessary. | ||
function addRevalidationParam( |
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 method handles our fine-grained revalidation query param. This should only show up on .data
requests if the app is has implemented shouldRevalidate on one or more of the matches
. Otherwise, we don't include a query param and while we may run a few extra loaders, the cacheability of the request should improve greatly resulting in a net-positive impact on server load.
packages/remix-react/single-fetch.ts
Outdated
let url = new URL(reqUrl); | ||
url.pathname = `${url.pathname === "/" ? "_root" : url.pathname}.data`; | ||
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.
single fetch URLs are /path/to/whatever.data
, except for the root route. To avoid a super odd-looking (and potentially invalid?) URL like /.data
, we use /_root.data
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.
.data
? I imagine it's not .json
because with RSC the response can be more than just JSON?
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.
Even now it's not JSON - it's the streaming format used by turbo-stream - so .data
is just a generic extension agnostic of the underlying streaming mechanism - which will eventually be RSC
This reverts commit faf262e.
Closes: #7641
RFC: #7640
Todo: