Ensure that Node.js polyfills are pre-optimized before the first request

.changeset/five-camels-cheer.md

@@ -0,0 +1,14 @@
+---
+"@cloudflare/vite-plugin": patch
+---
+
+Ensure that Node.js polyfills are pre-optimized before the first request
+
+Previously, these polyfills were only optimized on demand when Vite became aware of them.
+This was either because Vite was able to find an import to a polyfill when statically analysing the import tree of the entry-point,
+or when a polyfilled module was dynamically imported as part of a executing code to handle a request.
+
+In the second case, the optimizing of the dynamically imported dependency causes a reload of the Vite server, which can break applications that are holding state in modules during the request.
+This is the case of most React type frameworks, in particular React Router.
+
+Now, we pre-optimize all the possible Node.js polyfills when the server starts before the first request is handled.

.github/workflows/e2e.yml

@@ -43,11 +43,11 @@ jobs:
       - name: Run Vite E2E tests
         run: pnpm test:e2e -F @cloudflare/vite-plugin --log-order=stream
         env:
+          NODE_DEBUG: "vite-plugin:test"
+          # The AI tests need to connect to Cloudflare
           CLOUDFLARE_API_TOKEN: ${{ secrets.TEST_CLOUDFLARE_API_TOKEN }}
           CLOUDFLARE_ACCOUNT_ID: ${{ secrets.TEST_CLOUDFLARE_ACCOUNT_ID }}
           NODE_OPTIONS: "--max_old_space_size=8192"
-          WRANGLER_LOG_PATH: ${{ runner.temp }}/wrangler-debug-logs/
-          TEST_REPORT_PATH: ${{ runner.temp }}/test-report/index.html
           CI_OS: ${{ matrix.os }}
 
       - name: Run Wrangler E2E tests

packages/vite-plugin-cloudflare/e2e/README.md

@@ -25,17 +25,28 @@ The simplest test looks like:
 
 ```ts
 test("can serve a Worker request", async ({ expect, seed, viteDev }) => {
-	const projectPath = await seed("basic");
-	runCommand(`pnpm install`, projectPath);
+	const projectPath = await seed("basic", "pnpm");
 
 	const proc = await viteDev(projectPath);
 	const url = await waitForReady(proc);
 	expect(await fetchJson(url + "/api/")).toEqual({ name: "Cloudflare" });
 });
 ```
 
-- The `seed()` helper makes a copy of the named fixture into a temporary directory. It returns the path to the directory containing the copy (`projectPath` above). This directory will be deleted at the end of the test.
-- The `runCommand()` helper simply executes a one-shot command and resolves when it has exited. You can use this to install the dependencies of the fixture from the mock npm registry, as in the example above.
+- The `seed()` helper does the following:
+  - makes a copy of the named fixture into a temporary directory,
+  - updates the vite-plugin dependency in the package.json to match the local version
+  - runs `npm install` (or equivalent package manager command) in the temporary project
+  - returns the path to the directory containing the copy (`projectPath` above)
+  - the temporary directory will be deleted at the end of the test.
+- The `runCommand()` helper simply executes a one-shot command and resolves when it has exited. You can use this to install the dependencies of the fixture from the mock npm registry.
 - The `viteDev()` helper boots up the `vite dev` command and returns an object that can be used to monitor its output. The process will be killed at the end of the test.
 - The `waitForReady()` helper will resolve when the `vite dev` process has output its ready message, from which it will parse the url that can be fetched in the test.
 - The `fetchJson()` helper makes an Undici fetch to the url parsing the response into JSON. It will retry every 250ms for up to 10 secs to minimize flakes.
+
+## Debugging the tests
+
+You can provide the following environment variables to get access to the logs and the actual files being tested:
+
+- `NODE_DEBUG=vite-plugin:test` - this will display debugging log messages as well as the streamed output from the commands being run.
+- `CLOUDFLARE_VITE_E2E_KEEP_TEMP_DIRS=1` - this will prevent the temporary directory containing the test project from being deleted, so that you can go and play with it manually.

packages/vite-plugin-cloudflare/e2e/basic.test.ts

@@ -1,11 +1,10 @@
 import { describe } from "vitest";
-import { fetchJson, runCommand, test, waitForReady } from "./helpers.js";
+import { fetchJson, test, waitForReady } from "./helpers.js";
 
 describe("node compatibility", () => {
-	describe.each(["pnpm --no-store", "npm", "yarn"])("using %s", (pm) => {
+	describe.each(["pnpm", "npm", "yarn"])("using %s", (pm) => {
 		test("can serve a Worker request", async ({ expect, seed, viteDev }) => {
-			const projectPath = await seed("basic");
-			runCommand(`${pm} install`, projectPath);
+			const projectPath = await seed("basic", pm);
 
 			const proc = await viteDev(projectPath);
 			const url = await waitForReady(proc);
@@ -17,8 +16,7 @@ describe("node compatibility", () => {
 // This test checks that wrapped bindings which rely on additional workers with an authed connection to the CF API work
 describe("Workers AI", () => {
 	test("can serve a Worker request", async ({ expect, seed, viteDev }) => {
-		const projectPath = await seed("basic");
-		runCommand(`npm install`, projectPath);
+		const projectPath = await seed("basic", "npm");
 
 		const proc = await viteDev(projectPath);
 		const url = await waitForReady(proc);

packages/vite-plugin-cloudflare/e2e/dynamic.test.ts

@@ -0,0 +1,22 @@
+import { describe } from "vitest";
+import { fetchJson, test, waitForReady } from "./helpers.js";
+
+describe("prebundling Node.js compatibility", () => {
+	describe.each(["pnpm", "npm", "yarn"])("using %s", (pm) => {
+		test("will not cause a reload on a dynamic import of a Node.js module", async ({
+			expect,
+			seed,
+			viteDev,
+		}) => {
+			const projectPath = await seed("dynamic", pm);
+
+			const proc = await viteDev(projectPath);
+			const url = await waitForReady(proc);
+			expect(await fetchJson(url)).toEqual("OK!");
+			expect(proc.stdout).not.toContain(
+				"optimized dependencies changed. reloading"
+			);
+			expect(proc.stdout).not.toContain("[vite] program reload");
+		});
+	});
+});

packages/vite-plugin-cloudflare/e2e/fixtures/basic/package.json

@@ -1,5 +1,5 @@
 {
-	"name": "cloudflare-vite-tutorial",
+	"name": "cloudflare-vite-e2e-basic",
 	"version": "0.0.0",
 	"private": true,
 	"type": "module",

packages/vite-plugin-cloudflare/e2e/fixtures/basic/vite.config.ts

@@ -3,5 +3,5 @@ import react from "@vitejs/plugin-react";
 import { defineConfig } from "vite";
 
 export default defineConfig({
-	plugins: [react(), cloudflare()],
+	plugins: [react(), cloudflare({ inspectorPort: false, persistState: false })],
 });

packages/vite-plugin-cloudflare/e2e/fixtures/basic/wrangler.toml

@@ -1,4 +1,4 @@
-name = "cloudflare-vite-tutorial"
+name = "cloudflare-vite-e2e-basic"
 compatibility_date = "2024-12-30"
 compatibility_flags = [ "nodejs_compat" ]
 assets = { not_found_handling = "single-page-application", binding = "ASSETS" }

packages/vite-plugin-cloudflare/e2e/fixtures/dynamic/package.json

@@ -0,0 +1,25 @@
+{
+	"name": "cloudflare-vite-e2e-dynamic",
+	"version": "0.0.0",
+	"private": true,
+	"type": "module",
+	"scripts": {
+		"build": "vite build",
+		"dev": "vite",
+		"lint": "eslint .",
+		"preview": "vite preview"
+	},
+	"devDependencies": {
+		"@cloudflare/vite-plugin": "*",
+		"@cloudflare/workers-types": "^4.20250204.0",
+		"@eslint/js": "^9.19.0",
+		"eslint": "^9.19.0",
+		"eslint-plugin-react-hooks": "^5.0.0",
+		"eslint-plugin-react-refresh": "^0.4.18",
+		"globals": "^15.14.0",
+		"typescript": "~5.7.2",
+		"typescript-eslint": "^8.22.0",
+		"vite": "^6.1.0",
+		"wrangler": "^3.108.1"
+	}
+}

packages/vite-plugin-cloudflare/e2e/fixtures/dynamic/src/dynamic.ts

@@ -0,0 +1,8 @@
+// This dynamically imported module relies upon Node.js
+// When Vite becomes aware of this file it will need to optimize the `node:asset` library
+// if it hasn't already.
+
+import assert from "node:assert/strict";
+
+assert(true, "the world is broken!");
+export const x = '"OK!"';

packages/vite-plugin-cloudflare/e2e/fixtures/dynamic/src/index.ts

@@ -0,0 +1,8 @@
+export default {
+	async fetch() {
+		// By dynamically importing the `./dynamic` module we prevent Vite from being able to statically
+		// analyze the imports and pre-optimize the Node.js import that is within it.
+		const { x } = await import("./dynamic");
+		return new Response(x);
+	},
+} satisfies ExportedHandler;

packages/vite-plugin-cloudflare/e2e/fixtures/dynamic/tsconfig.json

@@ -0,0 +1,7 @@
+{
+	"files": [],
+	"references": [
+		{ "path": "./tsconfig.node.json" },
+		{ "path": "./tsconfig.worker.json" }
+	]
+}

packages/vite-plugin-cloudflare/e2e/fixtures/dynamic/tsconfig.node.json

@@ -0,0 +1,24 @@
+{
+	"compilerOptions": {
+		"tsBuildInfoFile": "./node_modules/.tmp/tsconfig.node.tsbuildinfo",
+		"target": "ES2022",
+		"lib": ["ES2023"],
+		"module": "ESNext",
+		"skipLibCheck": true,
+
+		/* Bundler mode */
+		"moduleResolution": "bundler",
+		"allowImportingTsExtensions": true,
+		"isolatedModules": true,
+		"moduleDetection": "force",
+		"noEmit": true,
+
+		/* Linting */
+		"strict": true,
+		"noUnusedLocals": true,
+		"noUnusedParameters": true,
+		"noFallthroughCasesInSwitch": true,
+		"noUncheckedSideEffectImports": true
+	},
+	"include": ["vite.config.ts"]
+}

packages/vite-plugin-cloudflare/e2e/fixtures/dynamic/tsconfig.worker.json

@@ -0,0 +1,8 @@
+{
+	"extends": "./tsconfig.node.json",
+	"compilerOptions": {
+		"tsBuildInfoFile": "./node_modules/.tmp/tsconfig.worker.tsbuildinfo",
+		"types": ["@cloudflare/workers-types/2023-07-01", "vite/client"]
+	},
+	"include": ["src"]
+}

packages/vite-plugin-cloudflare/e2e/fixtures/dynamic/vite.config.ts

@@ -0,0 +1,6 @@
+import { cloudflare } from "@cloudflare/vite-plugin";
+import { defineConfig } from "vite";
+
+export default defineConfig({
+	plugins: [cloudflare({ inspectorPort: false, persistState: false })],
+});

packages/vite-plugin-cloudflare/e2e/fixtures/dynamic/wrangler.toml

@@ -0,0 +1,4 @@
+name = "cloudflare-vite-e2e-dynamic"
+main = "./src/index.ts"
+compatibility_date = "2024-12-30"
+compatibility_flags = ["nodejs_compat"]

packages/vite-plugin-cloudflare/e2e/global-setup.ts

@@ -3,7 +3,7 @@ import os from "node:os";
 import path from "node:path";
 import util from "node:util";
 import { startMockNpmRegistry } from "@cloudflare/mock-npm-registry";
-import type { GlobalSetupContext } from "vitest/node";
+import type { TestProject } from "vitest/node";
 
 declare module "vitest" {
 	export interface ProvidedContext {
@@ -13,8 +13,7 @@ declare module "vitest" {
 
 // Using a global setup means we can modify tests without having to re-install
 // packages into our temporary directory
-// Typings for the GlobalSetupContext are augmented in `global-setup.d.ts`.
-export default async function ({ provide }: GlobalSetupContext) {
+export default async function ({ provide }: TestProject) {
 	const stopMockNpmRegistry = await startMockNpmRegistry(
 		"@cloudflare/vite-plugin"
 	);
@@ -28,7 +27,9 @@ export default async function ({ provide }: GlobalSetupContext) {
 	return async () => {
 		await stopMockNpmRegistry();
 
-		console.log("Cleaning up temporary directory...");
-		await fs.rm(root, { recursive: true, maxRetries: 10 });
+		if (!process.env.CLOUDFLARE_VITE_E2E_KEEP_TEMP_DIRS) {
+			console.log("Cleaning up temporary directory...", root);
+			await fs.rm(root, { recursive: true, maxRetries: 10 });
+		}
 	};
 }

packages/vite-plugin-cloudflare/e2e/helpers.ts

@@ -6,15 +6,28 @@ import util from "node:util";
 import { stripAnsi } from "miniflare";
 import kill from "tree-kill";
 import { test as baseTest, inject, vi } from "vitest";
+import vitePluginPackage from "../package.json";
 
 const debuglog = util.debuglog("vite-plugin:test");
 
+const testEnv = {
+	...process.env,
+	// The following env vars are set to ensure that package managers
+	// do not use the same global cache and accidentally hit race conditions.
+	YARN_CACHE_FOLDER: "./.yarn/cache",
+	YARN_ENABLE_GLOBAL_CACHE: "false",
+	PNPM_HOME: "./.pnpm",
+	npm_config_cache: "./.npm/cache",
+	// unset the VITEST env variable as this causes e2e issues with some frameworks
+	VITEST: undefined,
+};
+
 /**
  * Extends the Vitest `test()` function to support running vite in
  * well defined environments that represent real-world usage.
  */
 export const test = baseTest.extend<{
-	seed: (fixture: string) => Promise<string>;
+	seed: (fixture: string, pm: string) => Promise<string>;
 	viteDev: (
 		projectPath: string,
 		options?: { flags?: string[]; maxBuffer?: number }
@@ -24,13 +37,17 @@ export const test = baseTest.extend<{
 	async seed({}, use) {
 		const root = inject("root");
 		const projectPaths: string[] = [];
-		await use(async (fixture) => {
+		await use(async (fixture, pm) => {
 			const projectPath = path.resolve(root, fixture);
 			await fs.cp(path.resolve(__dirname, "fixtures", fixture), projectPath, {
 				recursive: true,
 				errorOnExist: true,
 			});
 			debuglog("Fixture copied to " + projectPath);
+			await updateVitePluginVersion(projectPath);
+			debuglog("Updated vite-plugin version in package.json");
+			runCommand(`${pm} install`, projectPath);
+			debuglog("Installed node modules");
 			projectPaths.push(projectPath);
 			return projectPath;
 		});
@@ -52,12 +69,28 @@ export const test = baseTest.extend<{
 			debuglog("starting vite for " + projectPath);
 			const proc = childProcess.exec(`pnpm exec vite dev`, {
 				cwd: projectPath,
+				env: testEnv,
 			});
 			processes.push(proc);
 			return wrap(proc);
 		});
 		debuglog("Closing down vite dev processes", processes.length);
-		processes.forEach((proc) => proc.pid && kill(proc.pid));
+		const result = await Promise.allSettled(
+			processes.map((proc) => {
+				return new Promise<number | undefined>((resolve, reject) => {
+					const pid = proc.pid;
+					if (!pid) {
+						resolve(undefined);
+					} else {
+						debuglog("killing process vite process", pid);
+						kill(pid, "SIGKILL", (error) =>
+							error ? reject(error) : resolve(pid)
+						);
+					}
+				});
+			})
+		);
+		debuglog("Killed processes", result);
 	},
 });
 
@@ -101,10 +134,28 @@ function wrap(proc: childProcess.ChildProcess): Process {
 	};
 }
 
+async function updateVitePluginVersion(projectPath: string) {
+	const pkg = JSON.parse(
+		await fs.readFile(path.resolve(projectPath, "package.json"), "utf8")
+	);
+	const fields = ["dependencies", "devDependencies", "peerDependencies"];
+	for (const field of fields) {
+		if (pkg[field]?.["@cloudflare/vite-plugin"]) {
+			pkg[field]["@cloudflare/vite-plugin"] = vitePluginPackage.version;
+		}
+	}
+	await fs.writeFile(
+		path.resolve(projectPath, "package.json"),
+		JSON.stringify(pkg, null, 2)
+	);
+}
+
 export function runCommand(command: string, cwd: string) {
+	debuglog("Running command:", command);
 	childProcess.execSync(command, {
 		cwd,
 		stdio: debuglog.enabled ? "inherit" : "ignore",
+		env: testEnv,
 	});
 }
 

packages/vite-plugin-cloudflare/e2e/tsconfig.json

@@ -5,7 +5,9 @@
 		"skipLibCheck": true,
 		"types": ["node", "vitest"],
 		"target": "ESNext",
-		"moduleResolution": "NodeNext",
-		"module": "NodeNext"
+		"moduleResolution": "Node",
+		"module": "ESNext",
+		"esModuleInterop": true,
+		"resolveJsonModule": true
 	}
 }