fix(vite): fix HMR preamble race condition (#7919)

Co-authored-by: Pedro Cattori <[email protected]>
remix-run · Nov 6, 2023 · 22869fe · 22869fe
1 parent fe0960a
commit 22869fe
Show file tree

Hide file tree

Showing 11 changed files with 28 additions and 52 deletions.
diff --git a/.changeset/afraid-suns-yawn.md b/.changeset/afraid-suns-yawn.md
@@ -0,0 +1,6 @@
+---
+"@remix-run/dev": patch
+"@remix-run/react": patch
+---
+
+Support rendering of `LiveReload` component after `Scripts` in Vite dev
diff --git a/.changeset/early-trees-walk.md b/.changeset/early-trees-walk.md
@@ -0,0 +1,6 @@
+---
+"@remix-run/dev": patch
+"@remix-run/react": patch
+---
+
+Support optional rendering of `LiveReload` component in Vite dev
diff --git a/docs/future/vite.md b/docs/future/vite.md
@@ -127,43 +127,6 @@ Vite handles imports for all sorts of different file types, sometimes in ways th
 + "include": ["env.d.ts", "**/*.ts", "**/*.tsx"],
 ```
 
-#### `LiveReload` before `Scripts`
-
-<docs-info>
-  This is a temporary workaround for a limitation that will be removed in the future.
-</docs-info>
-
-For React Fast Refresh to work, it [needs to be initialized before any app code is run][rfr-preamble].
-That means it needs to come _before_ your `<Scripts />` element that loads your app code.
-
-We're working on a better API that would eliminate issues with ordering scripts.
-But for now, you can work around this limitation by manually moving `<LiveReload />` before `<Scripts />`.
-If your app doesn't the `Scripts` component, you can safely ignore this step.
-
-👉 **Ensure `<LiveReload />` comes _before_ `<Scripts />`**
-
-```diff filename=app/root.tsx
-export default function App() {
-  return (
-    <html lang="en">
-      <head>
-        <meta charSet="utf-8" />
-        <meta name="viewport" content="width=device-width, initial-scale=1" />
-        <Meta />
-        <Links />
-      </head>
-      <body>
-        <Outlet />
-        <ScrollRestoration />
-+        <LiveReload />
-        <Scripts />
--        <LiveReload />
-      </body>
-    </html>
-  );
-}
-```
-
 #### Migrating from Remix App Server
 
 If you were using `remix-serve` in development (or `remix dev` without the `-c` flag), you'll need to switch to the new minimal dev server.
@@ -656,7 +619,6 @@ We're definitely late to the Vite party, but we're excited to be here now!
 [solidstart]: https://start.solidjs.com/getting-started/what-is-solidstart
 [sveltekit]: https://kit.svelte.dev/
 [supported-with-some-deprecations]: #mdx
-[rfr-preamble]: https://github.com/facebook/react/issues/16604#issuecomment-528663101
 [component-keys]: #component-keys
 [issues-vite]: https://github.com/remix-run/remix/labels/vite
 [hmr]: ../discussion/hot-module-replacement

diff --git a/integration/vite-css-dev-test.ts b/integration/vite-css-dev-test.ts
@@ -56,8 +56,8 @@ test.describe("Vite CSS dev", () => {
                   <div id="content">
                     <Outlet />
                   </div>
-                  <LiveReload />
                   <Scripts />
+                  <LiveReload />
                 </body>
               </html>
             );

diff --git a/integration/vite-dev-express-test.ts b/integration/vite-dev-express-test.ts
@@ -87,8 +87,8 @@ test.beforeAll(async () => {
                   <h1>Root</h1>
                   <Outlet />
                 </div>
-                <LiveReload />
                 <Scripts />
+                <LiveReload />
               </body>
             </html>
           );

diff --git a/integration/vite-dev-test.ts b/integration/vite-dev-test.ts
@@ -54,8 +54,8 @@ test.describe("Vite dev", () => {
                     <h1>Root</h1>
                     <Outlet />
                   </div>
-                  <LiveReload />
                   <Scripts />
+                  <LiveReload />
                 </body>
               </html>
             );

diff --git a/packages/remix-dev/manifest.ts b/packages/remix-dev/manifest.ts
@@ -20,7 +20,7 @@ export type Manifest = {
     };
   };
   hmr?: {
-    timestamp: number;
+    timestamp?: number;
     runtime: string;
   };
 };
diff --git a/packages/remix-dev/vite/plugin.ts b/packages/remix-dev/vite/plugin.ts
@@ -402,6 +402,9 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
     return {
       version: String(Math.random()),
       url: VirtualModule.url(browserManifestId),
+      hmr: {
+        runtime: VirtualModule.url(injectHmrRuntimeId),
+      },
       entry: {
         module: resolveFileUrl(pluginConfig, pluginConfig.entryClientFilePath),
         imports: [],
@@ -709,9 +712,9 @@ export const remixVitePlugin: RemixVitePlugin = (options = {}) => {
             'export * from "@remix-run/react";',
             'export const LiveReload = process.env.NODE_ENV !== "development" ? () => null : ',
             '() => createElement("script", {',
-            ' type: "module",',
-            " async: true,",
-            ` src: "${VirtualModule.url(injectHmrRuntimeId)}"`,
+            "  dangerouslySetInnerHTML: { ",
+            "    __html: `window.__remixLiveReloadEnabled = true`",
+            "  }",
             "});",
           ].join("\n");
         }
@@ -864,11 +867,10 @@ const inWebWorker = typeof WorkerGlobalScope !== 'undefined' && self instanceof
 let prevRefreshReg;
 let prevRefreshSig;
 
-if (import.meta.hot && !inWebWorker) {
+if (import.meta.hot && !inWebWorker && window.__remixLiveReloadEnabled) {
   if (!window.__vite_plugin_react_preamble_installed__) {
     throw new Error(
-      "@vitejs/plugin-react can't detect preamble. Something is wrong. " +
-      "See https://github.com/vitejs/vite-plugin-react/pull/11#discussion_r430879201"
+      "Remix Vite plugin can't detect preamble. Something is wrong."
     );
   }
 
@@ -881,7 +883,7 @@ if (import.meta.hot && !inWebWorker) {
 }`.replace(/\n+/g, "");
 
 const REACT_REFRESH_FOOTER = `
-if (import.meta.hot && !inWebWorker) {
+if (import.meta.hot && !inWebWorker && window.__remixLiveReloadEnabled) {
   window.$RefreshReg$ = prevRefreshReg;
   window.$RefreshSig$ = prevRefreshSig;
   RefreshRuntime.__hmr_import(import.meta.url).then((currentExports) => {

diff --git a/packages/remix-react/entry.ts b/packages/remix-react/entry.ts
@@ -38,7 +38,7 @@ export interface AssetsManifest {
   url: string;
   version: string;
   hmr?: {
-    timestamp: number;
+    timestamp?: number;
     runtime: string;
   };
 }
diff --git a/templates/unstable-vite-express/app/root.tsx b/templates/unstable-vite-express/app/root.tsx
@@ -25,8 +25,8 @@ export default function App() {
       <body>
         <Outlet />
         <ScrollRestoration />
-        <LiveReload />
         <Scripts />
+        <LiveReload />
       </body>
     </html>
   );

diff --git a/templates/unstable-vite/app/root.tsx b/templates/unstable-vite/app/root.tsx
@@ -25,8 +25,8 @@ export default function App() {
       <body>
         <Outlet />
         <ScrollRestoration />
-        <LiveReload />
         <Scripts />
+        <LiveReload />
       </body>
     </html>
   );