fix(vite-plugin-cloudflare): track server restart in module scope

.changeset/salty-plums-worry.md

@@ -0,0 +1,7 @@
+---
+"@cloudflare/vite-plugin": patch
+---
+
+fix: track server restart in module scope
+
+When using `@cloudflare/vite-plugin` with React Router, miniflare might be disposed during restart. This change makes sure to track when the dev server restart in module scope to avoid unexpected behavior.

packages/create-cloudflare/e2e/helpers/framework-helpers.ts

@@ -30,7 +30,6 @@ import { runC3 } from "./run-c3";
 import { kill, spawnWithLogging } from "./spawn";
 import type { TemplateConfig } from "../../src/templates";
 import type { RunnerConfig } from "./run-c3";
-import type { JsonMap } from "@iarna/toml";
 import type { Writable } from "stream";
 
 export type FrameworkTestConfig = RunnerConfig & {
@@ -87,33 +86,59 @@ export async function runC3ForFrameworkTest(
 	return match[1];
 }
 
-/**
- * Either update or create a wrangler configuration file to include a `TEST` var.
- *
- * This is rather than having a wrangler configuration file in the e2e test's fixture folder,
- * which overwrites any that comes from the framework's template.
- */
-export async function addTestVarsToWranglerToml(projectPath: string) {
+export function updateWranglerConfig(
+	projectPath: string,
+	// eslint-disable-next-line @typescript-eslint/no-explicit-any
+	handleUpdate: <T extends Record<string, any>>(config: T) => T,
+) {
 	const wranglerTomlPath = join(projectPath, "wrangler.toml");
 	const wranglerJsoncPath = join(projectPath, "wrangler.jsonc");
 
 	if (existsSync(wranglerTomlPath)) {
 		const wranglerToml = readToml(wranglerTomlPath);
-		wranglerToml.vars ??= {};
-		(wranglerToml.vars as JsonMap).TEST = "C3_TEST";
 
-		writeToml(wranglerTomlPath, wranglerToml);
+		writeToml(
+			wranglerTomlPath,
+			handleUpdate(JSON.parse(JSON.stringify(wranglerToml))),
+		);
+
+		return () => {
+			writeToml(wranglerTomlPath, handleUpdate(wranglerToml));
+		};
 	} else if (existsSync(wranglerJsoncPath)) {
 		const wranglerJsonc = readJSON(wranglerJsoncPath) as {
 			vars: Record<string, string>;
 		};
-		wranglerJsonc.vars ??= {};
-		wranglerJsonc.vars.TEST = "C3_TEST";
 
-		writeJSON(wranglerJsoncPath, wranglerJsonc);
+		writeJSON(
+			wranglerJsoncPath,
+			handleUpdate(JSON.parse(JSON.stringify(wranglerJsonc))),
+		);
+
+		return () => {
+			writeJSON(wranglerJsoncPath, wranglerJsonc);
+		};
 	}
 }
 
+/**
+ * Either update or create a wrangler configuration file to include a `TEST` var.
+ *
+ * This is rather than having a wrangler configuration file in the e2e test's fixture folder,
+ * which overwrites any that comes from the framework's template.
+ */
+export async function addTestVarsToWranglerToml(projectPath: string) {
+	updateWranglerConfig(projectPath, (config) => {
+		return {
+			...config,
+			vars: {
+				...config.vars,
+				TEST: "C3_TEST",
+			},
+		};
+	});
+}
+
 export async function verifyDeployment(
 	{ testCommitMessage }: FrameworkTestConfig,
 	frameworkId: string,
@@ -141,6 +166,83 @@ export async function verifyDeployment(
 	});
 }
 
+export async function verifyDevScript(
+	{ verifyDev }: FrameworkTestConfig,
+	{ devScript }: TemplateConfig,
+	projectPath: string,
+	logStream: Writable,
+) {
+	if (!verifyDev) {
+		return;
+	}
+
+	assert(
+		devScript,
+		"Expected a dev script as we are verifying the dev session in " +
+			projectPath,
+	);
+
+	// Run the dev-server on random ports to avoid colliding with other tests
+	const port = await getPort();
+	const proc = spawnWithLogging(
+		[
+			packageManager.name,
+			"run",
+			devScript,
+			...(packageManager.name === "npm" ? ["--"] : []),
+			...(verifyDev.devArgs ?? []),
+			"--port",
+			`${port}`,
+		],
+		{
+			cwd: projectPath,
+			env: {
+				VITEST: undefined,
+			},
+		},
+		logStream,
+	);
+
+	let restoreConfig: (() => void) | undefined;
+
+	try {
+		await retry(
+			{ times: 300, sleepMs: 5000 },
+			async () => await fetch(`http://127.0.0.1:${port}${verifyDev.route}`),
+		);
+
+		// Make a request to the specified test route
+		const res = await fetch(`http://127.0.0.1:${port}${verifyDev.route}`);
+		expect(await res.text()).toContain(verifyDev.expectedText);
+
+		if (verifyDev.configChanges) {
+			const { configChanges } = verifyDev;
+			const updatedVars = configChanges.vars;
+
+			restoreConfig = updateWranglerConfig(projectPath, (config) => ({
+				...config,
+				vars: {
+					...config.vars,
+					...updatedVars,
+				},
+			}));
+
+			await retry({ times: 10, sleepMs: 500 }, async () => {
+				const res2 = await fetch(`http://127.0.0.1:${port}${verifyDev.route}`);
+				expect(await res2.text()).toContain(configChanges.expectedText);
+			});
+		}
+	} finally {
+		// Kill the process gracefully so ports can be cleaned up
+		await kill(proc);
+		// Restore the wrangler config if we modified it
+		restoreConfig?.();
+		// Wait for a second to allow process to exit cleanly. Otherwise, the port might
+		// end up camped and cause future runs to fail
+		await sleep(1000);
+	}
+}
+
 export async function verifyPreviewScript(
 	{ verifyPreview }: FrameworkTestConfig,
 	{ previewScript }: TemplateConfig,

packages/create-cloudflare/e2e/helpers/run-c3.ts

@@ -46,6 +46,18 @@ export type RunnerConfig = {
 	 * Specifies whether to run the test script for the project and verify the exit code.
 	 */
 	verifyTest?: boolean;
+	/**
+	 * Specifies whether to run the dev script for the project and verify the response from the specified route.
+	 */
+	verifyDev?: {
+		devArgs?: string[];
+		route: string;
+		expectedText: string;
+		configChanges?: {
+			vars: Record<string, string>;
+			expectedText: string;
+		};
+	};
 };
 
 export const runC3 = async (

packages/create-cloudflare/e2e/tests/frameworks/frameworks.test.ts

@@ -17,6 +17,7 @@ import {
 	shouldRunTest,
 	testGitCommitMessage,
 	verifyDeployment,
+	verifyDevScript,
 	verifyPreviewScript,
 	verifyTypes,
 } from "../../helpers/framework-helpers";
@@ -134,6 +135,13 @@ describe
 							}
 						}
 
+						await verifyDevScript(
+							testConfig,
+							frameworkConfig,
+							project.path,
+							logStream,
+						);
+
 						await verifyPreviewScript(
 							testConfig,
 							frameworkConfig,

packages/create-cloudflare/e2e/tests/frameworks/test-config.ts

@@ -30,6 +30,17 @@ function getFrameworkTestConfig(pm: string): NamedFrameworkTestConfig[] {
 				expectedText: "Hello from Cloudflare",
 				previewArgs: ["--host=127.0.0.1"],
 			},
+			verifyDev: {
+				route: "/",
+				expectedText: "Hello from Cloudflare",
+				devArgs: ["--host=127.0.0.1"],
+				configChanges: {
+					vars: {
+						VALUE_FROM_CLOUDFLARE: "Hello React Router",
+					},
+					expectedText: "Hello React Router",
+				},
+			},
 			nodeCompat: false,
 			flags: ["--no-install", "--no-git-init"],
 		},

packages/mock-npm-registry/src/index.ts

@@ -41,9 +41,13 @@ export async function startMockNpmRegistry(...targetPackages: string[]) {
 	);
 
 	console.log(
-		`Starting up local npm registry on http://localhost:${registryPort} at ${registryPath}`
+		`Starting up local npm registry on http://localhost:${registryPort} at ${registryPath} with ${pkgs.size} packages published:`
 	);
 
+	for (const [pkg, pkgPath] of pkgs.entries()) {
+		console.log(` - ${pkg} (${pkgPath})`);
+	}
+
 	let stopServer = await startVerdaccioServer(configPath);
 
 	const tempConfig = path.join(registryPath, ".npmrc");

packages/vite-plugin-cloudflare/src/index.ts

@@ -64,6 +64,8 @@ const debuglog = util.debuglog("@cloudflare:vite-plugin");
 
 // this flag is used to show the workers configs warning only once
 let workersConfigsWarningShown = false;
+/** Used to track whether hooks are being called because of a server restart or a server close event. */
+let restartingServer = false;
 let miniflare: Miniflare | undefined;
 
 /**
@@ -76,8 +78,6 @@ let miniflare: Miniflare | undefined;
 export function cloudflare(pluginConfig: PluginConfig = {}): vite.Plugin[] {
 	const ctx = new PluginContext();
 	let containerImageTagsSeen = new Set<string>();
-	/** Used to track whether hooks are being called because of a server restart or a server close event. */
-	let restartingServer = false;
 
 	return [
 		{