Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Change hydration check from URL to matches #9695

Merged
merged 7 commits into from
Jul 24, 2024
Merged

Change hydration check from URL to matches #9695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cc29305
Change hydration check from URL to matches
brophdawg11 Jul 2, 2024
806733f
Fix unit test stub data
brophdawg11 Jul 2, 2024
92bf31b
Cleanups
brophdawg11 Jul 3, 2024
4ed05f8
Remove loop check to bring to a new PR
brophdawg11 Jul 24, 2024
5aabe98
Update changeset
brophdawg11 Jul 24, 2024
daa8a74
Updates
brophdawg11 Jul 24, 2024
cb8024f
Don't check in spa mode
brophdawg11 Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/tender-elephants-kneel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@remix-run/react": patch
"@remix-run/server-runtime": patch
---

- Change initial hydration route mismatch from a URL check to a matches check to be resistant to URL inconsistenceis
2 changes: 1 addition & 1 deletion packages/remix-react/__tests__/components-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ describe("<RemixServer>", () => {
describe("<RemixBrowser>", () => {
it("handles empty default export objects from the compiler", () => {
window.__remixContext = {
url: "/",
ssrMatches: ["root", "empty"],
state: {
loaderData: {},
},
Expand Down
54 changes: 30 additions & 24 deletions packages/remix-react/browser.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { initFogOfWar, useFogOFWarDiscovery } from "./fog-of-war";
/* eslint-disable prefer-let/prefer-let */
declare global {
var __remixContext: {
url: string;
ssrMatches: string[];
basename?: string;
state: HydrationState;
criticalCss?: string;
Expand Down Expand Up @@ -194,29 +194,6 @@ if (import.meta && import.meta.hot) {
*/
export function RemixBrowser(_props: RemixBrowserProps): ReactElement {
if (!router) {
// Hard reload if the path we tried to load is not the current path.
// This is usually the result of 2 rapid back/forward clicks from an
// external site into a Remix app, where we initially start the load for
// one URL and while the JS chunks are loading a second forward click moves
// us to a new URL. Avoid comparing search params because of CDNs which
// can be configured to ignore certain params and only pathname is relevant
// towards determining the route matches.
let initialPathname = window.__remixContext.url;
let hydratedPathname = window.location.pathname;
if (
initialPathname !== hydratedPathname &&
!window.__remixContext.isSpaMode
) {
let errorMsg =
`Initial URL (${initialPathname}) does not match URL at time of hydration ` +
`(${hydratedPathname}), reloading page...`;
console.error(errorMsg);
window.location.reload();
// Get out of here so the reload can happen - don't create the router
// since it'll then kick off unnecessary route.lazy() loads
return <></>;
}

// When single fetch is enabled, we need to suspend until the initial state
// snapshot is decoded into window.__remixContext.state
if (window.__remixContext.future.unstable_singleFetch) {
Expand Down Expand Up @@ -270,6 +247,35 @@ export function RemixBrowser(_props: RemixBrowserProps): ReactElement {
window.location,
window.__remixContext.basename
);

// Hard reload if the matches we rendered on the server aren't the matches
// we matched in the client, otherwise we'll try to hydrate without the
// right modules and throw a hydration error, which can put React into an
// infinite hydration loop when hydrating the full `<html>` document.
// This is usually the result of 2 rapid back/forward clicks from an
// external site into a Remix app, where we initially start the load for
// one URL and while the JS chunks are loading a second forward click moves
// us to a new URL.
let ssrMatches = window.__remixContext.ssrMatches;
let hasDifferentSSRMatches =
(initialMatches || []).length !== ssrMatches.length ||
!(initialMatches || []).every((m, i) => ssrMatches[i] === m.route.id);

if (hasDifferentSSRMatches && !window.__remixContext.isSpaMode) {
let ssr = ssrMatches.join(",");
let client = (initialMatches || []).map((m) => m.route.id).join(",");
let errorMsg =
`SSR Matches (${ssr}) do not match client matches (${client}) at ` +
`time of hydration , reloading page...`;
console.error(errorMsg);

window.location.reload();
Copy link
Contributor

@ngbrown ngbrown Aug 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This window reload seems to be creating an infinite reload loop on Remix 2.11.0 on a server-rendered 404 page. With or without a custom ErrorBoundary. See #9821.

It seems that the issue is because hasDifferentSSRMatches is set to true because (initialMatches || []).length !== ssrMatches.length is true. initialMatches is 0, because matchRoutes doesn't match a 404 to the root route? while ssrMatches is length 1 with the root route.

I patched it like this and this seems to work:

diff --git a/node_modules/@remix-run/react/dist/esm/browser.js b/node_modules/@remix-run/react/dist/esm/browser.js
index ac5bd95..461875b 100644
--- a/node_modules/@remix-run/react/dist/esm/browser.js
+++ b/node_modules/@remix-run/react/dist/esm/browser.js
@@ -163,11 +163,12 @@ function RemixBrowser(_props) {
       // us to a new URL.
       let ssrMatches = window.__remixContext.ssrMatches;
       let hasDifferentSSRMatches = (initialMatches || []).length !== ssrMatches.length || !(initialMatches || []).every((m, i) => ssrMatches[i] === m.route.id);
-      if (hasDifferentSSRMatches && !window.__remixContext.isSpaMode) {
+      if (hasDifferentSSRMatches && !window.__remixContext.isSpaMode && !(window.__remixContext.state.errors?.root?.status === 404)) {
         let ssr = ssrMatches.join(",");
         let client = (initialMatches || []).map(m => m.route.id).join(",");
         let errorMsg = `SSR Matches (${ssr}) do not match client matches (${client}) at ` + `time of hydration , reloading page...`;

The non-esm output at node_modules/@remix-run/react/dist/browser.js also needs the same patch.

Just testing for status === 404 probably isn't a complete answer if on a 404 the server also throws a 500 or something.


// Get out of here so the reload can happen - don't create the router
// since it'll then kick off unnecessary route.lazy() loads
return <></>;
}

if (initialMatches) {
for (let match of initialMatches) {
let routeId = match.route.id;
Expand Down
4 changes: 2 additions & 2 deletions packages/remix-server-runtime/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ async function handleDocumentRequest(
staticHandlerContext: context,
criticalCss,
serverHandoffString: createServerHandoffString({
url: context.location.pathname,
ssrMatches: context.matches.map((m) => m.route.id),
basename: build.basename,
criticalCss,
future: build.future,
Expand Down Expand Up @@ -592,7 +592,7 @@ async function handleDocumentRequest(
...entryContext,
staticHandlerContext: context,
serverHandoffString: createServerHandoffString({
url: context.location.pathname,
ssrMatches: context.matches.map((m) => m.route.id),
basename: build.basename,
future: build.future,
isSpaMode: build.isSpaMode,
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-server-runtime/serverHandoff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function createServerHandoffString<T>(serverHandoff: {
// we'd end up including duplicate info
state?: ValidateShape<T, HydrationState>;
criticalCss?: string;
url: string;
ssrMatches: string[];
basename: string | undefined;
future: FutureConfig;
isSpaMode: boolean;
Expand Down