Skip to content

Commit 28522ae

Browse files
Ensure that Node.js polyfills are pre-optimized before the first request (#8688)
* 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. * fixup! Ensure that Node.js polyfills are pre-optimized before the first request * add test * simplify the nodejs warnings plugin * fixup! Ensure that Node.js polyfills are pre-optimized before the first request * fixup! simplify the nodejs warnings plugin * add debugging to e2e tests * fixup! Ensure that Node.js polyfills are pre-optimized before the first request * Initialize nodeJsCompatWarnings in configResolved hook * Fix wrangler-configs-validation e2e tests --------- Co-authored-by: James Opstad <[email protected]>
1 parent ec1f813 commit 28522ae

File tree

27 files changed

+301
-103
lines changed

27 files changed

+301
-103
lines changed

.changeset/five-camels-cheer.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@cloudflare/vite-plugin": patch
3+
---
4+
5+
Ensure that Node.js polyfills are pre-optimized before the first request
6+
7+
Previously, these polyfills were only optimized on demand when Vite became aware of them.
8+
This was either because Vite was able to find an import to a polyfill when statically analysing the import tree of the entry-point,
9+
or when a polyfilled module was dynamically imported as part of a executing code to handle a request.
10+
11+
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.
12+
This is the case of most React type frameworks, in particular React Router.
13+
14+
Now, we pre-optimize all the possible Node.js polyfills when the server starts before the first request is handled.

.github/workflows/e2e.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ jobs:
4343
- name: Run Vite E2E tests
4444
run: pnpm test:e2e -F @cloudflare/vite-plugin --log-order=stream
4545
env:
46+
NODE_DEBUG: "vite-plugin:test"
47+
# The AI tests need to connect to Cloudflare
4648
CLOUDFLARE_API_TOKEN: ${{ secrets.TEST_CLOUDFLARE_API_TOKEN }}
4749
CLOUDFLARE_ACCOUNT_ID: ${{ secrets.TEST_CLOUDFLARE_ACCOUNT_ID }}
4850
NODE_OPTIONS: "--max_old_space_size=8192"
49-
WRANGLER_LOG_PATH: ${{ runner.temp }}/wrangler-debug-logs/
50-
TEST_REPORT_PATH: ${{ runner.temp }}/test-report/index.html
5151
CI_OS: ${{ matrix.os }}
5252

5353
- name: Run Wrangler E2E tests

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,28 @@ The simplest test looks like:
2525

2626
```ts
2727
test("can serve a Worker request", async ({ expect, seed, viteDev }) => {
28-
const projectPath = await seed("basic");
29-
runCommand(`pnpm install`, projectPath);
28+
const projectPath = await seed("basic", "pnpm");
3029

3130
const proc = await viteDev(projectPath);
3231
const url = await waitForReady(proc);
3332
expect(await fetchJson(url + "/api/")).toEqual({ name: "Cloudflare" });
3433
});
3534
```
3635

37-
- 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.
38-
- 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.
36+
- The `seed()` helper does the following:
37+
- makes a copy of the named fixture into a temporary directory,
38+
- updates the vite-plugin dependency in the package.json to match the local version
39+
- runs `npm install` (or equivalent package manager command) in the temporary project
40+
- returns the path to the directory containing the copy (`projectPath` above)
41+
- the temporary directory will be deleted at the end of the test.
42+
- 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.
3943
- 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.
4044
- 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.
4145
- 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.
46+
47+
## Debugging the tests
48+
49+
You can provide the following environment variables to get access to the logs and the actual files being tested:
50+
51+
- `NODE_DEBUG=vite-plugin:test` - this will display debugging log messages as well as the streamed output from the commands being run.
52+
- `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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import { describe } from "vitest";
2-
import { fetchJson, runCommand, test, waitForReady } from "./helpers.js";
2+
import { fetchJson, test, waitForReady } from "./helpers.js";
33

44
describe("node compatibility", () => {
5-
describe.each(["pnpm --no-store", "npm", "yarn"])("using %s", (pm) => {
5+
describe.each(["pnpm", "npm", "yarn"])("using %s", (pm) => {
66
test("can serve a Worker request", async ({ expect, seed, viteDev }) => {
7-
const projectPath = await seed("basic");
8-
runCommand(`${pm} install`, projectPath);
7+
const projectPath = await seed("basic", pm);
98

109
const proc = await viteDev(projectPath);
1110
const url = await waitForReady(proc);
@@ -17,8 +16,7 @@ describe("node compatibility", () => {
1716
// This test checks that wrapped bindings which rely on additional workers with an authed connection to the CF API work
1817
describe("Workers AI", () => {
1918
test("can serve a Worker request", async ({ expect, seed, viteDev }) => {
20-
const projectPath = await seed("basic");
21-
runCommand(`npm install`, projectPath);
19+
const projectPath = await seed("basic", "npm");
2220

2321
const proc = await viteDev(projectPath);
2422
const url = await waitForReady(proc);
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { describe } from "vitest";
2+
import { fetchJson, test, waitForReady } from "./helpers.js";
3+
4+
describe("prebundling Node.js compatibility", () => {
5+
describe.each(["pnpm", "npm", "yarn"])("using %s", (pm) => {
6+
test("will not cause a reload on a dynamic import of a Node.js module", async ({
7+
expect,
8+
seed,
9+
viteDev,
10+
}) => {
11+
const projectPath = await seed("dynamic", pm);
12+
13+
const proc = await viteDev(projectPath);
14+
const url = await waitForReady(proc);
15+
expect(await fetchJson(url)).toEqual("OK!");
16+
expect(proc.stdout).not.toContain(
17+
"optimized dependencies changed. reloading"
18+
);
19+
expect(proc.stdout).not.toContain("[vite] program reload");
20+
});
21+
});
22+
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
{
2-
"name": "cloudflare-vite-tutorial",
2+
"name": "cloudflare-vite-e2e-basic",
33
"version": "0.0.0",
44
"private": true,
55
"type": "module",

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,5 @@ import react from "@vitejs/plugin-react";
33
import { defineConfig } from "vite";
44

55
export default defineConfig({
6-
plugins: [react(), cloudflare()],
6+
plugins: [react(), cloudflare({ inspectorPort: false, persistState: false })],
77
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
name = "cloudflare-vite-tutorial"
1+
name = "cloudflare-vite-e2e-basic"
22
compatibility_date = "2024-12-30"
33
compatibility_flags = [ "nodejs_compat" ]
44
assets = { not_found_handling = "single-page-application", binding = "ASSETS" }
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{
2+
"name": "cloudflare-vite-e2e-dynamic",
3+
"version": "0.0.0",
4+
"private": true,
5+
"type": "module",
6+
"scripts": {
7+
"build": "vite build",
8+
"dev": "vite",
9+
"lint": "eslint .",
10+
"preview": "vite preview"
11+
},
12+
"devDependencies": {
13+
"@cloudflare/vite-plugin": "*",
14+
"@cloudflare/workers-types": "^4.20250204.0",
15+
"@eslint/js": "^9.19.0",
16+
"eslint": "^9.19.0",
17+
"eslint-plugin-react-hooks": "^5.0.0",
18+
"eslint-plugin-react-refresh": "^0.4.18",
19+
"globals": "^15.14.0",
20+
"typescript": "~5.7.2",
21+
"typescript-eslint": "^8.22.0",
22+
"vite": "^6.1.0",
23+
"wrangler": "^3.108.1"
24+
}
25+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// This dynamically imported module relies upon Node.js
2+
// When Vite becomes aware of this file it will need to optimize the `node:asset` library
3+
// if it hasn't already.
4+
5+
import assert from "node:assert/strict";
6+
7+
assert(true, "the world is broken!");
8+
export const x = '"OK!"';

0 commit comments

Comments
 (0)