Skip to content

Commit 497a957

Browse files
authored
feat: restart with development condition (#15291)
1 parent f75c89f commit 497a957

10 files changed

Lines changed: 259 additions & 8 deletions

File tree

integration/cli-test.ts

Lines changed: 172 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { spawnSync } from "node:child_process";
1+
import { spawn, spawnSync, type ChildProcess } from "node:child_process";
22
import {
33
copyFileSync,
44
existsSync,
@@ -14,6 +14,7 @@ import { fileURLToPath } from "node:url";
1414

1515
import { expect, test } from "@playwright/test";
1616
import dedent from "dedent";
17+
import getPort from "get-port";
1718
import semver from "semver";
1819

1920
import { build, createProject, reactRouterConfig } from "./helpers/vite";
@@ -22,6 +23,7 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url));
2223
const rootDirectory = path.resolve(__dirname, "..");
2324
const nodeBin = process.argv[0];
2425
const reactRouterBin = "node_modules/@react-router/dev/dist/cli/index.js";
26+
const reactRouterPackageBinPath = "node_modules/@react-router/dev/bin.cjs";
2527
const reactRouterPackageBin = path.join(
2628
rootDirectory,
2729
"packages/react-router-dev/bin.cjs",
@@ -30,6 +32,121 @@ const reactRouterPackageBin = path.join(
3032
const run = (command: string[], options: Parameters<typeof spawnSync>[2]) =>
3133
spawnSync(nodeBin, [reactRouterBin, ...command], options);
3234

35+
function bufferize(stream: NodeJS.ReadableStream | null): () => string {
36+
let buffer = "";
37+
stream?.on("data", (data) => (buffer += data.toString()));
38+
return () => buffer;
39+
}
40+
41+
function delay(ms: number) {
42+
return new Promise((resolve) => setTimeout(resolve, ms));
43+
}
44+
45+
function restartCount(output: string) {
46+
return (
47+
output.match(/\[restart\] Relaunching with NODE_OPTIONS:/g)?.length ?? 0
48+
);
49+
}
50+
51+
function getLogs(stdout: string, stderr: string) {
52+
return [
53+
`stdout:\n${stdout || "<empty>"}`,
54+
`stderr:\n${stderr || "<empty>"}`,
55+
].join("\n\n");
56+
}
57+
58+
async function waitForDevServer(args: {
59+
port: number;
60+
proc: ChildProcess;
61+
stdout: () => string;
62+
stderr: () => string;
63+
}) {
64+
let timeout = process.platform === "win32" ? 20_000 : 10_000;
65+
let start = Date.now();
66+
let lastError: unknown;
67+
68+
while (Date.now() - start < timeout) {
69+
let stdout = args.stdout();
70+
let stderr = args.stderr();
71+
72+
if (restartCount(stdout) > 1) {
73+
throw new Error(
74+
`Expected react-router dev to restart once, but it restarted ${restartCount(
75+
stdout,
76+
)} times.\n\n${getLogs(stdout, stderr)}`,
77+
);
78+
}
79+
80+
if (args.proc.exitCode !== null || args.proc.signalCode !== null) {
81+
throw new Error(
82+
`react-router dev exited before the server started.\n\n${getLogs(
83+
stdout,
84+
stderr,
85+
)}`,
86+
);
87+
}
88+
89+
try {
90+
let response = await fetch(`http://127.0.0.1:${args.port}/`, {
91+
signal: AbortSignal.timeout(1_000),
92+
});
93+
let html = await response.text();
94+
if (response.ok && html.includes("Welcome to React Router")) {
95+
return;
96+
}
97+
lastError = new Error(`Unexpected response ${response.status}: ${html}`);
98+
} catch (error) {
99+
lastError = error;
100+
}
101+
102+
await delay(100);
103+
}
104+
105+
throw new Error(
106+
[
107+
`Timed out waiting for react-router dev to start: ${String(lastError)}`,
108+
getLogs(args.stdout(), args.stderr()),
109+
].join("\n\n"),
110+
);
111+
}
112+
113+
function waitForExit(proc: ChildProcess, timeout: number) {
114+
return new Promise<{ code: number | null; signal: NodeJS.Signals | null }>(
115+
(resolve, reject) => {
116+
if (proc.exitCode !== null || proc.signalCode !== null) {
117+
resolve({ code: proc.exitCode, signal: proc.signalCode });
118+
return;
119+
}
120+
121+
let timer = setTimeout(() => {
122+
reject(new Error("Timed out waiting for react-router dev to exit"));
123+
}, timeout);
124+
125+
proc.once("exit", (code, signal) => {
126+
clearTimeout(timer);
127+
resolve({ code, signal });
128+
});
129+
},
130+
);
131+
}
132+
133+
function killProcessGroup(proc: ChildProcess) {
134+
if (proc.exitCode !== null || proc.signalCode !== null) {
135+
return;
136+
}
137+
138+
if (proc.pid && process.platform !== "win32") {
139+
try {
140+
process.kill(-proc.pid, "SIGKILL");
141+
return;
142+
} catch {
143+
// Fall back to killing just the parent process below.
144+
}
145+
}
146+
147+
proc.kill("SIGKILL");
148+
}
149+
33150
const getBinNodeEnv = (command: string[]) => {
34151
let cwd = mkdtempSync(path.join(tmpdir(), "react-router-bin-"));
35152
let env = { ...process.env };
@@ -162,6 +279,60 @@ test.describe("cli", () => {
162279
);
163280
});
164281

282+
test("dev restarts with the development condition and starts the server", async ({
283+
browserName: _browserName,
284+
}, { project }) => {
285+
test.skip(
286+
project.name !== "chromium",
287+
"CLI smoke test only needs one browser project",
288+
);
289+
290+
let cwd = await createProject();
291+
let port = await getPort();
292+
let proc = spawn(
293+
nodeBin,
294+
[
295+
reactRouterPackageBinPath,
296+
"dev",
297+
"--host",
298+
"127.0.0.1",
299+
"--port",
300+
String(port),
301+
"--strictPort",
302+
],
303+
{
304+
cwd,
305+
detached: process.platform !== "win32",
306+
env: {
307+
...process.env,
308+
FORCE_COLOR: undefined,
309+
NO_COLOR: "1",
310+
NODE_OPTIONS: "--no-warnings=ExperimentalWarning",
311+
},
312+
stdio: "pipe",
313+
},
314+
);
315+
let stdout = bufferize(proc.stdout);
316+
let stderr = bufferize(proc.stderr);
317+
318+
try {
319+
await waitForDevServer({ port, proc, stdout, stderr });
320+
expect(restartCount(stdout())).toBe(1);
321+
322+
proc.kill("SIGTERM");
323+
await expect(waitForExit(proc, 5_000)).resolves.toBeDefined();
324+
} catch (error) {
325+
throw new Error(
326+
[
327+
error instanceof Error ? error.message : String(error),
328+
getLogs(stdout(), stderr()),
329+
].join("\n\n"),
330+
);
331+
} finally {
332+
killProcessGroup(proc);
333+
}
334+
});
335+
165336
test("routes", async () => {
166337
const cwd = await createProject();
167338
let { stdout, stderr, status } = run(["routes"], { cwd });
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- Restart `react-router dev` with `--conditions=development` when not enabled
2+
-

packages/react-router-dev/cli/commands.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import colors from "picocolors";
88
// Workaround for "ERR_REQUIRE_CYCLE_MODULE" in Node 22.10.0+
99
import "react-router";
1010

11+
import developmentConditionEnabled from "#development-condition-enabled";
1112
import type { ViteDevOptions } from "../vite/dev";
1213
import type { ViteBuildOptions } from "../vite/build";
1314
import { hasNodeDependency, loadConfig } from "../config/config";
@@ -18,6 +19,7 @@ import * as profiler from "../vite/profiler";
1819
import * as Typegen from "../typegen";
1920
import { preloadVite, getVite } from "../vite/vite";
2021
import { hasReactRouterRscPlugin } from "../vite/has-rsc-plugin";
22+
import { restartWithMergedOptions } from "../restart-with-conditions";
2123

2224
const nodeRequire = createRequire(import.meta.url);
2325

@@ -62,14 +64,18 @@ export async function build(
6264
}
6365

6466
export async function dev(root?: string, options: ViteDevOptions = {}) {
65-
let { dev } = await import("../vite/dev");
66-
if (options.profile) {
67-
await profiler.start();
68-
}
69-
exitHook(() => profiler.stop(console.info));
67+
if (developmentConditionEnabled) {
68+
let { dev } = await import("../vite/dev");
69+
if (options.profile) {
70+
await profiler.start();
71+
}
72+
exitHook(() => profiler.stop(console.info));
7073

71-
root = resolveRootDirectory(root, options);
72-
await dev(root, options);
74+
root = resolveRootDirectory(root, options);
75+
await dev(root, options);
76+
} else {
77+
restartWithMergedOptions("--conditions=development");
78+
}
7379

7480
// keep `react-router dev` alive by waiting indefinitely
7581
await new Promise(() => {});
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
declare const developmentConditionEnabled: boolean;
2+
export default developmentConditionEnabled;
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
const developmentConditionEnabled = false;
2+
export default developmentConditionEnabled;
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
declare const developmentConditionEnabled: boolean;
2+
export default developmentConditionEnabled;
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
const developmentConditionEnabled = true;
2+
export default developmentConditionEnabled;

packages/react-router-dev/package.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@
4040
"./package.json": "./package.json"
4141
},
4242
"imports": {
43+
"#development-condition-enabled": {
44+
"development": "./development-condition-enabled/true.mjs",
45+
"default": "./development-condition-enabled/false.mjs"
46+
},
4347
"#module-sync-enabled": {
4448
"module-sync": "./module-sync-enabled/true.mjs",
4549
"default": "./module-sync-enabled/false.cjs"
@@ -70,6 +74,7 @@
7074
"../../pnpm-workspace.yaml",
7175
"cli/**",
7276
"config/**",
77+
"development-condition-enabled/**",
7378
"module-sync-enabled/**",
7479
"typegen/**",
7580
"vite/**",
@@ -165,6 +170,7 @@
165170
},
166171
"files": [
167172
"dist/",
173+
"development-condition-enabled/",
168174
"module-sync-enabled/",
169175
"bin.cjs",
170176
"rsc-types.d.ts",
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { spawn, type ChildProcess } from "node:child_process";
2+
import process from "node:process";
3+
4+
/**
5+
* Restarts the current Node process, appending new flags to the
6+
* existing NODE_OPTIONS. SIGINT/SIGTERM are always forwarded to the child.
7+
*/
8+
export function restartWithMergedOptions(nodeOptions: string): void {
9+
if (process.env.REACT_ROUTER_DEV_RESTARTED === "true") {
10+
throw new Error(
11+
"restartWithMergedOptions() was called, but the process has already been restarted. This is likely a bug in @react-router/dev."
12+
);
13+
}
14+
const mergedOptions = [process.env.NODE_OPTIONS, nodeOptions]
15+
.filter(Boolean)
16+
.join(" ")
17+
.trim();
18+
19+
console.log(`[restart] Relaunching with NODE_OPTIONS: ${mergedOptions}`);
20+
21+
const [cmd, ...args] = process.argv;
22+
23+
const child: ChildProcess = spawn(cmd, args, {
24+
env: {
25+
...process.env,
26+
NODE_OPTIONS: mergedOptions,
27+
REACT_ROUTER_DEV_RESTARTED: "true",
28+
},
29+
stdio: "inherit",
30+
});
31+
32+
const signals: NodeJS.Signals[] = ["SIGINT", "SIGTERM"];
33+
let signalHandlers = signals.map((sig) => {
34+
let handler = () => {
35+
child.kill(sig);
36+
};
37+
process.on(sig, handler);
38+
return [sig, handler] as const;
39+
});
40+
41+
child.on("exit", (code, signal) => {
42+
for (let [sig, handler] of signalHandlers) {
43+
process.off(sig, handler);
44+
}
45+
46+
if (signal) {
47+
process.kill(process.pid, signal);
48+
} else {
49+
process.exit(code ?? 0);
50+
}
51+
});
52+
53+
child.on("error", (err) => {
54+
console.error("[restart] Failed to spawn child process:", err);
55+
process.exit(1);
56+
});
57+
}

packages/react-router-dev/tsdown.config.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import pkg from "./package.json" with { type: "json" };
1010
const entry = ["cli/index.ts", "config.ts", "routes.ts", "vite.ts"];
1111

1212
const neverBundle = [
13+
"#development-condition-enabled",
1314
"./static/refresh-utils.mjs",
1415
"./static/rsc-refresh-utils.mjs",
1516
/\.json$/,

0 commit comments

Comments
 (0)