Skip to content

Commit 1a2bbf8

Browse files
jamesopstadjamesopstad
authored
Set default values when statically replacing process.env.NODE_ENV (#11027)
* Add failing test * Remove process.env.NODE_ENV condition for applying define * Add node-env fixture * Correctly replace process.env.NODE_ENV in development and production * Also replace global.process.env.NODE_ENV and globalThis.process.env.NODE_ENV * Add changeset * PR feedback
1 parent daa3529 commit 1a2bbf8

File tree

10 files changed

+346
-94
lines changed

10 files changed

+346
-94
lines changed

‎.changeset/common-colts-clap.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"wrangler": minor
3+
---
4+
5+
Statically replace the value of `process.env.NODE_ENV` with `development` for development builds and `production` for production builds if it is not set. Else, use the given value. This ensures that libraries, such as React, that branch code based on `process.env.NODE_ENV` can be properly tree shaken.

‎fixtures/node-env/package.json‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
{
2+
"name": "@fixture/node-env",
3+
"private": true,
4+
"scripts": {
5+
"check:type": "tsc",
6+
"dev": "wrangler dev",
7+
"test:ci": "vitest run",
8+
"test:watch": "vitest"
9+
},
10+
"dependencies": {
11+
"react": "^18.3.1",
12+
"react-dom": "^18.3.1"
13+
},
14+
"devDependencies": {
15+
"@cloudflare/workers-tsconfig": "workspace:*",
16+
"@cloudflare/workers-types": "catalog:default",
17+
"@types/node": "catalog:default",
18+
"@types/react": "^18.3.3",
19+
"@types/react-dom": "^18.2.0",
20+
"miniflare": "workspace:*",
21+
"typescript": "catalog:default",
22+
"vitest": "catalog:default",
23+
"wrangler": "workspace:*"
24+
},
25+
"volta": {
26+
"extends": "../../package.json"
27+
}
28+
}

‎fixtures/node-env/src/index.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import React from "react";
2+
import { renderToString } from "react-dom/server";
3+
4+
export default {
5+
async fetch(request) {
6+
const url = new URL(request.url);
7+
8+
if (url.pathname === "/ssr") {
9+
const content = renderToString(
10+
React.createElement("h1", null, "Hello world")
11+
);
12+
13+
return new Response(content);
14+
}
15+
16+
return new Response(
17+
`The value of process.env.NODE_ENV is "${process.env.NODE_ENV}"`
18+
);
19+
},
20+
} satisfies ExportedHandler;

‎fixtures/node-env/tests/node-env.test.ts‎

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import { spawnSync } from "node:child_process";
2+
import * as fs from "node:fs";
3+
import * as path from "node:path";
4+
import { Miniflare } from "miniflare";
5+
import { describe, it, vi } from "vitest";
6+
import { runWranglerDev } from "../../shared/src/run-wrangler-long-lived";
7+
8+
describe("`process.env.NODE_ENV` replacement in development", () => {
9+
it("replaces `process.env.NODE_ENV` with `development` if it is `undefined`", async ({
10+
expect,
11+
}) => {
12+
vi.stubEnv("NODE_ENV", undefined);
13+
14+
const { ip, port, stop } = await runWranglerDev(
15+
path.resolve(__dirname, ".."),
16+
["--port=0", "--inspector-port=0"]
17+
);
18+
19+
await vi.waitFor(async () => {
20+
const response = await fetch(`http://${ip}:${port}/`);
21+
const text = await response.text();
22+
expect(text).toBe(`The value of process.env.NODE_ENV is "development"`);
23+
});
24+
25+
await stop();
26+
27+
vi.unstubAllEnvs();
28+
});
29+
30+
it("replaces `process.env.NODE_ENV` with the given value if it is set", async ({
31+
expect,
32+
}) => {
33+
vi.stubEnv("NODE_ENV", "some-value");
34+
35+
const { ip, port, stop } = await runWranglerDev(
36+
path.resolve(__dirname, ".."),
37+
["--port=0", "--inspector-port=0"]
38+
);
39+
40+
await vi.waitFor(async () => {
41+
const response = await fetch(`http://${ip}:${port}/`);
42+
const text = await response.text();
43+
expect(text).toBe(`The value of process.env.NODE_ENV is "some-value"`);
44+
});
45+
46+
await stop();
47+
48+
vi.unstubAllEnvs();
49+
});
50+
});
51+
52+
describe("`process.env.NODE_ENV` replacement in production", () => {
53+
const url = "http://localhost";
54+
55+
it("replaces `process.env.NODE_ENV` with `production` if it is `undefined`", async ({
56+
expect,
57+
}) => {
58+
vi.stubEnv("NODE_ENV", undefined);
59+
60+
spawnSync("npx wrangler build", {
61+
shell: true,
62+
stdio: "pipe",
63+
});
64+
65+
const miniflare = new Miniflare({
66+
modules: [
67+
{
68+
type: "ESModule",
69+
path: "./dist/index.js",
70+
},
71+
],
72+
});
73+
74+
await miniflare.ready;
75+
76+
await vi.waitFor(async () => {
77+
const response = await miniflare.dispatchFetch(url);
78+
const text = await response.text();
79+
expect(text).toBe(`The value of process.env.NODE_ENV is "production"`);
80+
});
81+
82+
await miniflare.dispose();
83+
84+
vi.unstubAllEnvs();
85+
});
86+
87+
it("replaces `process.env.NODE_ENV` with the given value if it is set", async ({
88+
expect,
89+
}) => {
90+
vi.stubEnv("NODE_ENV", "some-value");
91+
92+
spawnSync("npx wrangler build", {
93+
shell: true,
94+
stdio: "pipe",
95+
});
96+
97+
const miniflare = new Miniflare({
98+
modules: [
99+
{
100+
type: "ESModule",
101+
path: "./dist/index.js",
102+
},
103+
],
104+
});
105+
106+
await miniflare.ready;
107+
108+
await vi.waitFor(async () => {
109+
const response = await miniflare.dispatchFetch(url);
110+
const text = await response.text();
111+
expect(text).toBe(`The value of process.env.NODE_ENV is "some-value"`);
112+
});
113+
114+
await miniflare.dispose();
115+
116+
vi.unstubAllEnvs();
117+
});
118+
119+
it("tree shakes React when `process.env.NODE_ENV` is `production`", ({
120+
expect,
121+
}) => {
122+
vi.stubEnv("NODE_ENV", undefined);
123+
124+
spawnSync("npx wrangler build", {
125+
shell: true,
126+
stdio: "pipe",
127+
});
128+
129+
const outputJs = fs.readFileSync("./dist/index.js", "utf8");
130+
131+
expect(outputJs).not.toContain("react-dom.development.js");
132+
// the React development code links to the facebook/react repo
133+
expect(outputJs).not.toContain("https://github.com/facebook/react");
134+
135+
vi.unstubAllEnvs();
136+
});
137+
});

‎fixtures/node-env/tsconfig.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"extends": "@cloudflare/workers-tsconfig/tsconfig.json",
3+
"compilerOptions": {
4+
"types": ["@cloudflare/workers-types/experimental", "node"]
5+
},
6+
"include": ["src", "tests"]
7+
}

‎fixtures/node-env/vitest.config.mts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { defineProject, mergeConfig } from "vitest/config";
2+
import configShared from "../../vitest.shared";
3+
4+
export default mergeConfig(
5+
configShared,
6+
defineProject({
7+
test: {},
8+
})
9+
);

‎fixtures/node-env/wrangler.jsonc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "node-env",
3+
"compatibility_date": "2025-10-01",
4+
"main": "./src/index.ts",
5+
}

‎packages/wrangler/src/__tests__/navigator-user-agent.test.ts‎

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ import assert from "node:assert";
22
import { mkdir, readFile, writeFile } from "node:fs/promises";
33
import path from "node:path";
44
import dedent from "ts-dedent";
5+
import { vi } from "vitest";
56
import { bundleWorker } from "../deployment-bundle/bundle";
67
import { noopModuleCollector } from "../deployment-bundle/module-collection";
78
import { isNavigatorDefined } from "../navigator-user-agent";
89
import { mockConsoleMethods } from "./helpers/mock-console";
910
import { runInTempDir } from "./helpers/run-in-tmp";
11+
import type { BundleOptions } from "../deployment-bundle/bundle";
1012

1113
/*
1214
* This file contains inline comments with the word "javascript"
@@ -82,8 +84,25 @@ describe("isNavigatorDefined", () => {
8284
describe("defineNavigatorUserAgent is respected", () => {
8385
runInTempDir();
8486
const std = mockConsoleMethods();
87+
const sharedBundleOptions = {
88+
bundle: true,
89+
additionalModules: [],
90+
moduleCollector: noopModuleCollector,
91+
doBindings: [],
92+
workflowBindings: [],
93+
define: {},
94+
alias: {},
95+
checkFetch: false,
96+
targetConsumer: "deploy",
97+
local: true,
98+
projectRoot: process.cwd(),
99+
} satisfies Partial<BundleOptions>;
100+
101+
afterEach(() => {
102+
vi.unstubAllEnvs();
103+
});
85104

86-
it("defineNavigatorUserAgent = false, navigator preserved", async () => {
105+
it("preserves `navigator` when `defineNavigatorUserAgent` is `false`", async () => {
87106
await seedFs({
88107
"src/index.js": dedent/* javascript */ `
89108
function randomBytes(length) {
@@ -113,17 +132,7 @@ describe("defineNavigatorUserAgent is respected", () => {
113132
path.resolve("dist"),
114133
// @ts-expect-error Ignore the requirement for passing undefined values
115134
{
116-
bundle: true,
117-
additionalModules: [],
118-
moduleCollector: noopModuleCollector,
119-
doBindings: [],
120-
workflowBindings: [],
121-
define: {},
122-
alias: {},
123-
checkFetch: false,
124-
targetConsumer: "deploy",
125-
local: true,
126-
projectRoot: process.cwd(),
135+
...sharedBundleOptions,
127136
defineNavigatorUserAgent: false,
128137
}
129138
);
@@ -145,7 +154,7 @@ describe("defineNavigatorUserAgent is respected", () => {
145154
expect(fileContents).toContain("navigator.userAgent");
146155
});
147156

148-
it("defineNavigatorUserAgent = true, navigator treeshaken", async () => {
157+
it("tree shakes `navigator` when `defineNavigatorUserAgent` is `true`", async () => {
149158
await seedFs({
150159
"src/index.js": dedent/* javascript */ `
151160
function randomBytes(length) {
@@ -175,17 +184,53 @@ describe("defineNavigatorUserAgent is respected", () => {
175184
path.resolve("dist"),
176185
// @ts-expect-error Ignore the requirement for passing undefined values
177186
{
178-
bundle: true,
179-
additionalModules: [],
180-
moduleCollector: noopModuleCollector,
181-
doBindings: [],
182-
workflowBindings: [],
183-
define: {},
184-
alias: {},
185-
checkFetch: false,
186-
targetConsumer: "deploy",
187-
local: true,
187+
...sharedBundleOptions,
188+
defineNavigatorUserAgent: true,
189+
}
190+
);
191+
192+
// Build time warning is suppressed, because esbuild treeshakes the relevant code path
193+
expect(std.warn).toMatchInlineSnapshot(`""`);
194+
195+
const fileContents = await readFile("dist/index.js", "utf8");
196+
197+
// navigator.userAgent should have been defined, and so should not be present in the bundle
198+
expect(fileContents).not.toContain("navigator.userAgent");
199+
});
200+
201+
it("tree shakes `navigator` when `defineNavigatorUserAgent` is `true` and `process.env.NODE_ENV` is `undefined`", async () => {
202+
await seedFs({
203+
"src/index.js": dedent/* javascript */ `
204+
function randomBytes(length) {
205+
if (navigator.userAgent !== "Cloudflare-Workers") {
206+
return new Uint8Array(require("node:crypto").randomBytes(length));
207+
} else {
208+
return crypto.getRandomValues(new Uint8Array(length));
209+
}
210+
}
211+
export default {
212+
async fetch(request, env) {
213+
return new Response(randomBytes(10))
214+
},
215+
};
216+
`,
217+
});
218+
219+
vi.stubEnv("NODE_ENV", undefined);
220+
221+
await bundleWorker(
222+
{
223+
file: path.resolve("src/index.js"),
188224
projectRoot: process.cwd(),
225+
configPath: undefined,
226+
format: "modules",
227+
moduleRoot: path.dirname(path.resolve("src/index.js")),
228+
exports: [],
229+
},
230+
path.resolve("dist"),
231+
// @ts-expect-error Ignore the requirement for passing undefined values
232+
{
233+
...sharedBundleOptions,
189234
defineNavigatorUserAgent: true,
190235
}
191236
);

‎packages/wrangler/src/deployment-bundle/bundle.ts‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,13 @@ export async function bundleWorker(
352352
},
353353
};
354354

355+
const nodeEnvReplacement = JSON.stringify(
356+
// use process.env["NODE_ENV" + ""] so that esbuild doesn't replace it
357+
// when we do a build of wrangler. (re: https://github.com/cloudflare/workers-sdk/issues/1477)
358+
process.env["NODE_ENV" + ""] ||
359+
(targetConsumer === "deploy" ? "production" : "development")
360+
);
361+
355362
const buildOptions = {
356363
// Don't use entryFile here as the file may have been changed when applying the middleware
357364
entryPoints: [entry.file],
@@ -383,17 +390,15 @@ export async function bundleWorker(
383390
metafile: true,
384391
conditions: getBuildConditions(),
385392
platform: getBuildPlatform(),
386-
...(process.env.NODE_ENV && {
387-
define: {
388-
...(defineNavigatorUserAgent
389-
? { "navigator.userAgent": `"Cloudflare-Workers"` }
390-
: {}),
391-
// use process.env["NODE_ENV" + ""] so that esbuild doesn't replace it
392-
// when we do a build of wrangler. (re: https://github.com/cloudflare/workers-sdk/issues/1477)
393-
"process.env.NODE_ENV": `"${process.env["NODE_ENV" + ""]}"`,
394-
...define,
395-
},
396-
}),
393+
define: {
394+
...(defineNavigatorUserAgent
395+
? { "navigator.userAgent": `"Cloudflare-Workers"` }
396+
: {}),
397+
"process.env.NODE_ENV": nodeEnvReplacement,
398+
"global.process.env.NODE_ENV": nodeEnvReplacement,
399+
"globalThis.process.env.NODE_ENV": nodeEnvReplacement,
400+
...define,
401+
},
397402
loader: COMMON_ESBUILD_OPTIONS.loader,
398403
plugins: [
399404
aliasPlugin,

0 commit comments

Comments
 (0)