-
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
Fix HDR for single fetch #9954
Fix HDR for single fetch #9954
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@remix-run/react": patch | ||
--- | ||
|
||
[REMOVE] Fix HDR for single fetch |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,7 +58,13 @@ const enqueueUpdate = debounce(async () => { | |
window.__remixRouteModuleUpdates.clear(); | ||
} | ||
|
||
await revalidate(); | ||
try { | ||
window.__remixHdrActive = true; | ||
await revalidate(); | ||
} finally { | ||
window.__remixHdrActive = false; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to know during the single fetch |
||
|
||
if (manifest) { | ||
Object.assign(window.__remixManifest, manifest); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -261,46 +261,48 @@ async function singleFetchLoaderNavigationStrategy( | |
results[m.route.id] = { type: "error", result: e }; | ||
} | ||
return; | ||
} else if (!manifest.routes[m.route.id].hasLoader) { | ||
// If we don't have a server loader, then we don't care about the HTTP | ||
// call and can just send back a `null` - because we _do_ have a `loader` | ||
// in the client router handling route module/styles loads | ||
results[m.route.id] = { | ||
type: "data", | ||
result: null, | ||
}; | ||
Comment on lines
-268
to
-271
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was unnecessary - should instead be assigning to |
||
return; | ||
} | ||
|
||
// Otherwise, we want to load this route on the server and can lump this | ||
// it in with the others on a singular promise | ||
routesParams.add(m.route.id); | ||
// Load this route on the server if it has a loader | ||
if (manifest.routes[m.route.id].hasLoader) { | ||
routesParams.add(m.route.id); | ||
} | ||
|
||
await handler(async () => { | ||
try { | ||
// Lump this match in with the others on a singular promise | ||
try { | ||
let result = await handler(async () => { | ||
let data = await singleFetchDfd.promise; | ||
results[m.route.id] = { | ||
type: "data", | ||
result: unwrapSingleFetchResults(data, m.route.id), | ||
}; | ||
} catch (e) { | ||
results[m.route.id] = { | ||
type: "error", | ||
result: e, | ||
}; | ||
} | ||
}); | ||
return unwrapSingleFetchResults(data, m.route.id); | ||
}); | ||
results[m.route.id] = { | ||
type: "data", | ||
result, | ||
}; | ||
return result; | ||
} catch (e) { | ||
results[m.route.id] = { | ||
type: "error", | ||
result: e, | ||
}; | ||
} | ||
}) | ||
) | ||
); | ||
|
||
// Wait for all routes to resolve above before we make the HTTP call | ||
await routesLoadedPromise; | ||
|
||
// Don't make any single fetch server calls: | ||
// We can skip the server call: | ||
// - On initial hydration - only clientLoaders can pass through via `clientLoader.hydrate` | ||
// - If there are no routes to fetch from the server | ||
if (!router.state.initialized || routesParams.size === 0) { | ||
// | ||
// One exception - if we are performing an HDR revalidation we have to call | ||
// the server in case a new loader has shown up that the manifest doesn't yet | ||
// know about | ||
if ( | ||
(!router.state.initialized || routesParams.size === 0) && | ||
!window.__remixHdrActive | ||
) { | ||
singleFetchDfd.resolve({}); | ||
} else { | ||
try { | ||
|
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.
Didn't have copies of these tests with the flag enabled so didn't catch this until we migrated the changes to RR v7 where single fetch is always on