fix(dev): dev server listens for updated build hash before writing se…

…rver files previously there was a race where the app server could respond with the updated manifest version (aka build hash) _before_ the dev server was listening for that version.
remix-run · May 3, 2023 · 895b403 · 895b403
1 parent acdd7c7
commit 895b403
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 35 deletions.
diff --git a/.changeset/poor-nails-warn.md b/.changeset/poor-nails-warn.md
@@ -0,0 +1,8 @@
+---
+"@remix-run/dev": patch
+---
+
+fix race where app server responds with updated manifest version _before_ dev server is listening for it
+
+dev server now listens for updated versions _before_ writing the server changes, guaranteeing that it is listening
+before the app server gets a chance to send its 'ready' message
diff --git a/packages/remix-dev/compiler/compiler.ts b/packages/remix-dev/compiler/compiler.ts
@@ -10,7 +10,9 @@ import { create as createManifest, write as writeManifest } from "./manifest";
 import { err, ok } from "../result";
 
 type Compiler = {
-  compile: () => Promise<Manifest>;
+  compile: (options?: {
+    onManifest?: (manifest: Manifest) => void;
+  }) => Promise<Manifest>;
   cancel: () => Promise<void>;
   dispose: () => Promise<void>;
 };
@@ -43,7 +45,9 @@ export let create = async (ctx: Context): Promise<Compiler> => {
     ]);
   };
 
-  let compile = async () => {
+  let compile = async (
+    options: { onManifest?: (manifest: Manifest) => void } = {}
+  ) => {
     let error: unknown | undefined = undefined;
     let errCancel = (thrown: unknown) => {
       if (error === undefined) {
@@ -102,6 +106,7 @@ export let create = async (ctx: Context): Promise<Compiler> => {
       hmr,
     });
     channels.manifest.ok(manifest);
+    options.onManifest?.(manifest);
     writes.manifest = writeManifest(ctx.config, manifest);
 
     // server compilation

diff --git a/packages/remix-dev/compiler/watch.ts b/packages/remix-dev/compiler/watch.ts
@@ -4,11 +4,11 @@ import * as path from "path";
 
 import type { RemixConfig } from "../config";
 import { readConfig } from "../config";
-import { type Manifest } from "../manifest";
 import * as Compiler from "./compiler";
 import type { Context } from "./context";
 import { logThrown } from "./utils/log";
 import { normalizeSlashes } from "../config/routes";
+import type { Manifest } from "../manifest";
 
 function isEntryPoint(config: RemixConfig, file: string): boolean {
   let appFile = path.relative(config.appDirectory, file);
@@ -24,7 +24,8 @@ function isEntryPoint(config: RemixConfig, file: string): boolean {
 export type WatchOptions = {
   reloadConfig?(root: string): Promise<RemixConfig>;
   onBuildStart?(ctx: Context): void;
-  onBuildFinish?(ctx: Context, durationMs: number, manifest?: Manifest): void;
+  onBuildManifest?(manifest: Manifest): void;
+  onBuildFinish?(ctx: Context, durationMs: number, ok: boolean): void;
   onFileCreated?(file: string): void;
   onFileChanged?(file: string): void;
   onFileDeleted?(file: string): void;
@@ -35,6 +36,7 @@ export async function watch(
   {
     reloadConfig = readConfig,
     onBuildStart,
+    onBuildManifest,
     onBuildFinish,
     onFileCreated,
     onFileChanged,
@@ -44,15 +46,15 @@ export async function watch(
   let start = Date.now();
   let compiler = await Compiler.create(ctx);
   let compile = () =>
-    compiler.compile().catch((thrown) => {
+    compiler.compile({ onManifest: onBuildManifest }).catch((thrown) => {
       logThrown(thrown);
       return undefined;
     });
 
   // initial build
   onBuildStart?.(ctx);
   let manifest = await compile();
-  onBuildFinish?.(ctx, Date.now() - start, manifest);
+  onBuildFinish?.(ctx, Date.now() - start, manifest !== undefined);
 
   let restart = debounce(async () => {
     onBuildStart?.(ctx);
@@ -68,14 +70,14 @@ export async function watch(
 
     compiler = await Compiler.create(ctx);
     let manifest = await compile();
-    onBuildFinish?.(ctx, Date.now() - start, manifest);
+    onBuildFinish?.(ctx, Date.now() - start, manifest !== undefined);
   }, 500);
 
   let rebuild = debounce(async () => {
     onBuildStart?.(ctx);
     let start = Date.now();
     let manifest = await compile();
-    onBuildFinish?.(ctx, Date.now() - start, manifest);
+    onBuildFinish?.(ctx, Date.now() - start, manifest !== undefined);
   }, 100);
 
   let toWatch = [ctx.config.appDirectory];

diff --git a/packages/remix-dev/devServer_unstable/index.ts b/packages/remix-dev/devServer_unstable/index.ts
@@ -68,10 +68,10 @@ export let serve = async (
   };
 
   let state: {
-    latestBuildHash?: string;
-    buildHashChannel?: Channel.Type<void>;
     appServer?: execa.ExecaChildProcess;
+    manifest?: Manifest;
     prevManifest?: Manifest;
+    appReady?: Channel.Type<void>;
   } = {};
 
   let bin = await detectBin();
@@ -101,8 +101,8 @@ export let serve = async (
               if (matches) {
                 for (let match of matches) {
                   let buildHash = match[1];
-                  if (buildHash === state.latestBuildHash) {
-                    state.buildHashChannel?.ok();
+                  if (buildHash === state.manifest?.version) {
+                    state.appReady?.ok();
                   }
                 }
               }
@@ -134,46 +134,52 @@ export let serve = async (
         return patchPublicPath(config, httpOrigin);
       },
       onBuildStart: (ctx) => {
-        state.buildHashChannel?.err();
+        state.appReady?.err();
         clean(ctx.config);
         websocket.log(state.prevManifest ? "Rebuilding..." : "Building...");
       },
-      onBuildFinish: async (ctx, durationMs, manifest) => {
-        if (!manifest) return;
+      onBuildManifest: (manifest: Manifest) => {
+        state.manifest = manifest;
+      },
+      onBuildFinish: async (ctx, durationMs, succeeded) => {
+        if (!succeeded) return;
 
         websocket.log(
           (state.prevManifest ? "Rebuilt" : "Built") +
             ` in ${prettyMs(durationMs)}`
         );
-        let prevManifest = state.prevManifest;
-        state.prevManifest = manifest;
-        state.latestBuildHash = manifest.version;
-        state.buildHashChannel = Channel.create();
+        state.appReady = Channel.create();
 
         let start = Date.now();
-        console.log(`Waiting for app server (${state.latestBuildHash})`);
+        console.log(`Waiting for app server (${state.manifest?.version})`);
         if (
           options.command &&
           (state.appServer === undefined || options.restart)
         ) {
           await kill(state.appServer);
           state.appServer = startAppServer(options.command);
         }
-        let { ok } = await state.buildHashChannel.result;
+        let { ok } = await state.appReady.result;
         // result not ok -> new build started before this one finished. do not process outdated manifest
-        if (!ok) return;
-        console.log(`App server took ${prettyMs(Date.now() - start)}`);
-
-        if (manifest.hmr && prevManifest) {
-          let updates = HMR.updates(ctx.config, manifest, prevManifest);
-          websocket.hmr(manifest, updates);
-
-          let hdr = updates.some((u) => u.revalidate);
-          console.log("> HMR" + (hdr ? " + HDR" : ""));
-        } else if (prevManifest !== undefined) {
-          websocket.reload();
-          console.log("> Live reload");
+        if (ok) {
+          console.log(`App server took ${prettyMs(Date.now() - start)}`);
+
+          if (state.manifest?.hmr && state.prevManifest) {
+            let updates = HMR.updates(
+              ctx.config,
+              state.manifest,
+              state.prevManifest
+            );
+            websocket.hmr(state.manifest, updates);
+
+            let hdr = updates.some((u) => u.revalidate);
+            console.log("> HMR" + (hdr ? " + HDR" : ""));
+          } else if (state.prevManifest !== undefined) {
+            websocket.reload();
+            console.log("> Live reload");
+          }
         }
+        state.prevManifest = state.manifest;
       },
       onFileCreated: (file) =>
         websocket.log(`File created: ${relativePath(file)}`),
@@ -207,8 +213,8 @@ export let serve = async (
         console.warn(`Unrecognized payload: ${req.body}`);
         res.sendStatus(400);
       }
-      if (buildHash === state.latestBuildHash) {
-        state.buildHashChannel?.ok();
+      if (buildHash === state.manifest?.version) {
+        state.appReady?.ok();
       }
       res.sendStatus(200);
     })