diff --git a/.changeset/strong-dolphins-tell.md b/.changeset/strong-dolphins-tell.md new file mode 100644 index 00000000000..3e109e2b71e --- /dev/null +++ b/.changeset/strong-dolphins-tell.md @@ -0,0 +1,5 @@ +--- +"@remix-run/dev": patch +--- + +fix dev server crashes caused by ungraceful hdr error handling diff --git a/integration/hmr-log-test.ts b/integration/hmr-log-test.ts index cdb02556ed4..d7aa1dfd672 100644 --- a/integration/hmr-log-test.ts +++ b/integration/hmr-log-test.ts @@ -463,6 +463,49 @@ whatsup fs.writeFileSync(mdxPath, originalMdx); await page.waitForSelector(`#crazy`); expect(dataRequests).toBe(5); + + // dev server doesn't crash when rebuild fails + await page.click(`a[href="/"]`); + await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); + await page.waitForLoadState("networkidle"); + + expect(devStderr()).toBe(""); + let withSyntaxError = ` + import { useLoaderData } from "@remix-run/react"; + export function shouldRevalidate(args) { + return true; + } + eport efault functio Index() { + const t = useLoaderData(); + return ( + +

With Syntax Error

+ + ) + } + `; + fs.writeFileSync(indexPath, withSyntaxError); + await wait(() => devStderr().includes('Expected ";" but found "efault"'), { + timeoutMs: HMR_TIMEOUT_MS, + }); + + let withFix = ` + import { useLoaderData } from "@remix-run/react"; + export function shouldRevalidate(args) { + return true; + } + export default function Index() { + // const t = useLoaderData(); + return ( +
+

With Fix

+
+ ) + } + `; + fs.writeFileSync(indexPath, withFix); + await page.waitForLoadState("networkidle"); + await page.getByText("With Fix").waitFor({ timeout: HMR_TIMEOUT_MS }); } catch (e) { console.log("stdout begin -----------------------"); console.log(devStdout()); diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index b3c418906e7..e369070b9d0 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -462,6 +462,49 @@ whatsup fs.writeFileSync(mdxPath, originalMdx); await page.waitForSelector(`#crazy`); expect(dataRequests).toBe(5); + + // dev server doesn't crash when rebuild fails + await page.click(`a[href="/"]`); + await page.getByText("Hello, planet").waitFor({ timeout: HMR_TIMEOUT_MS }); + await page.waitForLoadState("networkidle"); + + expect(devStderr()).toBe(""); + let withSyntaxError = ` + import { useLoaderData } from "@remix-run/react"; + export function shouldRevalidate(args) { + return true; + } + eport efault functio Index() { + const t = useLoaderData(); + return ( + +

With Syntax Error

+ + ) + } + `; + fs.writeFileSync(indexPath, withSyntaxError); + await wait(() => devStderr().includes('Expected ";" but found "efault"'), { + timeoutMs: HMR_TIMEOUT_MS, + }); + + let withFix = ` + import { useLoaderData } from "@remix-run/react"; + export function shouldRevalidate(args) { + return true; + } + export default function Index() { + // const t = useLoaderData(); + return ( +
+

With Fix

+
+ ) + } + `; + fs.writeFileSync(indexPath, withFix); + await page.waitForLoadState("networkidle"); + await page.getByText("With Fix").waitFor({ timeout: HMR_TIMEOUT_MS }); } catch (e) { console.log("stdout begin -----------------------"); console.log(devStdout()); diff --git a/packages/remix-dev/devServer_unstable/hdr.ts b/packages/remix-dev/devServer_unstable/hdr.ts index bacb6475338..69e859b2493 100644 --- a/packages/remix-dev/devServer_unstable/hdr.ts +++ b/packages/remix-dev/devServer_unstable/hdr.ts @@ -16,7 +16,9 @@ function isBareModuleId(id: string): boolean { type Route = Context["config"]["routes"][string]; -export let detectLoaderChanges = async (ctx: Context) => { +export let detectLoaderChanges = async ( + ctx: Context +): Promise> => { let entryPoints: Record = {}; for (let id of Object.keys(ctx.config.routes)) { entryPoints[id] = ctx.config.routes[id].file + "?loader"; @@ -30,6 +32,7 @@ export let detectLoaderChanges = async (ctx: Context) => { write: false, entryNames: "[hash]", loader: loaders, + logLevel: "silent", plugins: [ { name: "hmr-loader", @@ -98,13 +101,13 @@ export let detectLoaderChanges = async (ctx: Context) => { }; let { metafile } = await esbuild.build(options); - let entries = Object.entries(metafile!.outputs).map( - ([hashjs, { entryPoint }]) => { - let file = entryPoint - ?.replace(/^hmr-loader:/, "") - ?.replace(/\?loader$/, ""); - return [file, hashjs.replace(/\.js$/, "")]; - } - ); - return Object.fromEntries(entries); + + let entries: Record = {}; + for (let [hashjs, { entryPoint }] of Object.entries(metafile!.outputs)) { + if (entryPoint === undefined) continue; + let file = entryPoint.replace(/^hmr-loader:/, "").replace(/\?loader$/, ""); + entries[file] = hashjs.replace(/\.js$/, ""); + } + + return entries; }; diff --git a/packages/remix-dev/devServer_unstable/index.ts b/packages/remix-dev/devServer_unstable/index.ts index c3d0e0a2fb5..e4492d350a9 100644 --- a/packages/remix-dev/devServer_unstable/index.ts +++ b/packages/remix-dev/devServer_unstable/index.ts @@ -15,6 +15,8 @@ import * as HMR from "./hmr"; import { warnOnce } from "../warnOnce"; import { detectPackageManager } from "../cli/detectPackageManager"; import * as HDR from "./hdr"; +import type { Result } from "../result"; +import { err, ok } from "../result"; type Origin = { scheme: string; @@ -59,7 +61,7 @@ export let serve = async ( manifest?: Manifest; prevManifest?: Manifest; appReady?: Channel.Type; - hdr?: Promise>; + loaderChanges?: Promise>>; prevLoaderHashes?: Record; } = {}; @@ -122,57 +124,70 @@ export let serve = async ( }, { onBuildStart: async (ctx) => { + // stop listening for previous manifest state.appReady?.err(); + clean(ctx.config); websocket.log(state.prevManifest ? "Rebuilding..." : "Building..."); - state.hdr = HDR.detectLoaderChanges(ctx); + state.loaderChanges = HDR.detectLoaderChanges(ctx).then(ok, err); }, onBuildManifest: (manifest: Manifest) => { state.manifest = manifest; + state.appReady = Channel.create(); }, onBuildFinish: async (ctx, durationMs, succeeded) => { if (!succeeded) return; - websocket.log( (state.prevManifest ? "Rebuilt" : "Built") + ` in ${prettyMs(durationMs)}` ); - state.appReady = Channel.create(); - let start = Date.now(); - 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.appReady.result; - // result not ok -> new build started before this one finished. do not process outdated manifest - let loaderHashes = await state.hdr; - if (ok) { + // accumulate new state, but only update state after updates are processed + let newState: typeof state = { prevManifest: state.manifest }; + try { + console.log(`Waiting for app server (${state.manifest?.version})`); + let start = Date.now(); + if ( + options.command && + (state.appServer === undefined || options.restart) + ) { + await kill(state.appServer); + state.appServer = startAppServer(options.command); + } + let appReady = await state.appReady!.result; + if (!appReady.ok) return; console.log(`App server took ${prettyMs(Date.now() - start)}`); - if (state.manifest && loaderHashes && state.prevManifest) { + + // HMR + HDR + let loaderChanges = await state.loaderChanges!; + if (loaderChanges.ok) { + newState.prevLoaderHashes = loaderChanges.value; + } + if (loaderChanges?.ok && state.manifest && state.prevManifest) { let updates = HMR.updates( ctx.config, state.manifest, state.prevManifest, - loaderHashes, + loaderChanges.value, state.prevLoaderHashes ); websocket.hmr(state.manifest, updates); let hdr = updates.some((u) => u.revalidate); console.log("> HMR" + (hdr ? " + HDR" : "")); - } else if (state.prevManifest !== undefined) { + return; + } + + // Live Reload + if (state.prevManifest !== undefined) { websocket.reload(); console.log("> Live reload"); } + } finally { + // commit accumulated state + Object.assign(state, newState); } - state.prevManifest = state.manifest; - state.prevLoaderHashes = loaderHashes; }, onFileCreated: (file) => websocket.log(`File created: ${relativePath(file)}`),