Skip to content

Commit

Permalink
fix(dev): gracefully handle hdr errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@pcattori
pcattori committed May 24, 2023
1 parent c90c16e commit 895c3db
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 32 deletions.
5 changes: 5 additions & 0 deletions .changeset/strong-dolphins-tell.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": patch
---

fix dev server crashes caused by ungraceful hdr error handling
43 changes: 43 additions & 0 deletions integration/hmr-log-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<mai>
<h1>With Syntax Error</h1>
</main>
)
}
`;
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 (
<main>
<h1>With Fix</h1>
</main>
)
}
`;
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());
Expand Down
43 changes: 43 additions & 0 deletions integration/hmr-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<mai>
<h1>With Syntax Error</h1>
</main>
)
}
`;
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 (
<main>
<h1>With Fix</h1>
</main>
)
}
`;
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());
Expand Down
23 changes: 13 additions & 10 deletions packages/remix-dev/devServer_unstable/hdr.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Record<string, string>> => {
let entryPoints: Record<string, string> = {};
for (let id of Object.keys(ctx.config.routes)) {
entryPoints[id] = ctx.config.routes[id].file + "?loader";
Expand All @@ -30,6 +32,7 @@ export let detectLoaderChanges = async (ctx: Context) => {
write: false,
entryNames: "[hash]",
loader: loaders,
logLevel: "silent",
plugins: [
{
name: "hmr-loader",
Expand Down Expand Up @@ -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<string, string> = {};
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;
};
59 changes: 37 additions & 22 deletions packages/remix-dev/devServer_unstable/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -59,7 +61,7 @@ export let serve = async (
manifest?: Manifest;
prevManifest?: Manifest;
appReady?: Channel.Type<void>;
hdr?: Promise<Record<string, string>>;
loaderChanges?: Promise<Result<Record<string, string>>>;
prevLoaderHashes?: Record<string, string>;
} = {};

Expand Down Expand Up @@ -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)}`),
Expand Down

0 comments on commit 895c3db

Please sign in to comment.