From 70df4230a0622dc83787696c7b2d19c5b44a6ddc Mon Sep 17 00:00:00 2001 From: Josiah Grace Date: Tue, 4 Jul 2023 15:52:02 -0700 Subject: [PATCH 1/4] Guard usage of router with a promise for hmr --- packages/remix-react/browser.tsx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/remix-react/browser.tsx b/packages/remix-react/browser.tsx index 3bad7c70855..af38de51f6a 100644 --- a/packages/remix-react/browser.tsx +++ b/packages/remix-react/browser.tsx @@ -48,6 +48,15 @@ declare global { let router: Router; let hmrAbortController: AbortController | undefined; +let hmrRouterReadyResolve: ((router: Router) => void) | undefined; +// There's a race condition with HMR where the remix:manifest is signaled before +// the router is assigned in the RemixBrowser component. This promise gates the +// HMR handler until the router is ready +let hmrRouterReadyPromise = new Promise((resolve) => { + // body of a promise is executed immediately, so this can be resolved outside + // of the promise body + hmrRouterReadyResolve = resolve; +}); if (import.meta && import.meta.hot) { import.meta.hot.accept( @@ -59,6 +68,7 @@ if (import.meta && import.meta.hot) { assetsManifest: EntryContext["manifest"]; needsRevalidation: Set; }) => { + const router = await hmrRouterReadyPromise; let routeIds = [ ...new Set( router.state.matches @@ -180,6 +190,7 @@ export function RemixBrowser(_props: RemixBrowserProps): ReactElement { window.__remixContext.future.v2_normalizeFormMethod, }, }); + hmrRouterReadyResolve?.(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 From 005b18641b49863c8d6ce199af6f094b9cc79e4f Mon Sep 17 00:00:00 2001 From: Josiah Grace Date: Tue, 4 Jul 2023 16:18:14 -0700 Subject: [PATCH 2/4] Sign CLA --- contributors.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/contributors.yml b/contributors.yml index ecb1d1a4cca..007e61056b3 100644 --- a/contributors.yml +++ b/contributors.yml @@ -555,3 +555,4 @@ - mrkhosravian - tanerijun - ally1002 +- defjosiah From aba7ca623fbf596eeb84abfa2f83e700ffc1d928 Mon Sep 17 00:00:00 2001 From: Josiah Grace Date: Tue, 4 Jul 2023 16:22:01 -0700 Subject: [PATCH 3/4] Add changeset --- .changeset/mean-months-lick.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/mean-months-lick.md diff --git a/.changeset/mean-months-lick.md b/.changeset/mean-months-lick.md new file mode 100644 index 00000000000..8d3feedeb8c --- /dev/null +++ b/.changeset/mean-months-lick.md @@ -0,0 +1,5 @@ +--- +"@remix-run/react": patch +--- + +fix router race condition for hmr From 59552f49d9c2722718d781a078a22ff26f410720 Mon Sep 17 00:00:00 2001 From: Jacob Ebey Date: Wed, 5 Jul 2023 12:18:20 -0700 Subject: [PATCH 4/4] - handle promise rejection - notify after reload check to avoid any extra pending HMR work --- packages/remix-react/browser.tsx | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/remix-react/browser.tsx b/packages/remix-react/browser.tsx index af38de51f6a..65b394692cb 100644 --- a/packages/remix-react/browser.tsx +++ b/packages/remix-react/browser.tsx @@ -56,6 +56,10 @@ let hmrRouterReadyPromise = new Promise((resolve) => { // body of a promise is executed immediately, so this can be resolved outside // of the promise body hmrRouterReadyResolve = resolve; +}).catch(() => { + // This is a noop catch handler to avoid unhandled promise rejection warnings + // in the console. The promise is never rejected. + return undefined; }); if (import.meta && import.meta.hot) { @@ -68,7 +72,15 @@ if (import.meta && import.meta.hot) { assetsManifest: EntryContext["manifest"]; needsRevalidation: Set; }) => { - const router = await hmrRouterReadyPromise; + let router = await hmrRouterReadyPromise; + // This should never happen, but just in case... + if (!router) { + console.error( + "Failed to accept HMR update because the router was not ready." + ); + return; + } + let routeIds = [ ...new Set( router.state.matches @@ -190,8 +202,7 @@ export function RemixBrowser(_props: RemixBrowserProps): ReactElement { window.__remixContext.future.v2_normalizeFormMethod, }, }); - hmrRouterReadyResolve?.(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 @@ -208,6 +219,11 @@ export function RemixBrowser(_props: RemixBrowserProps): ReactElement { console.error(errorMsg); window.location.reload(); } + + // Notify that the router is ready for HMR + if (hmrRouterReadyResolve) { + hmrRouterReadyResolve(router); + } } let [location, setLocation] = React.useState(router.state.location);