Improve DX with node:* modules

.changeset/chatty-balloons-impress.md

@@ -0,0 +1,44 @@
+---
+"wrangler": minor
+---
+
+feat: Support runtime-agnostic polyfills
+
+Previously, Wrangler treated any imports of `node:*` modules as build-time errors (unless one of the two Node.js compatibility modes was enabled). This is sometimes overly aggressive, since those imports are often not hit at runtime (for instance, it was impossible to write a library that worked across Node.JS and Workers, using Node packages only when running in Node). Here's an example of a function that would cause Wrangler to fail to build:
+
+```ts
+export function randomBytes(length: number) {
+	if (navigator.userAgent !== "Cloudflare-Workers") {
+		return new Uint8Array(require("node:crypto").randomBytes(length));
+	} else {
+		return crypto.getRandomValues(new Uint8Array(length));
+	}
+}
+```
+
+This function _should_ work in both Workers and Node, since it gates Node-specific functionality behind a user agent check, and falls back to the built-in Workers crypto API. Instead, Wrangler detected the `node:crypto` import and failed with the following error:
+
+```
+✘ [ERROR] Could not resolve "node:crypto"
+
+    src/randomBytes.ts:5:36:
+      5 │ ... return new Uint8Array(require('node:crypto').randomBytes(length));
+        ╵                                   ~~~~~~~~~~~~~
+
+  The package "node:crypto" wasn't found on the file system but is built into node.
+  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.
+```
+
+This change turns that Wrangler build failure into a warning, which users can choose to ignore if they know the import of `node:*` APIs is safe (because it will never trigger at runtime, for instance):
+
+```
+▲ [WARNING] The package "node:crypto" wasn't found on the file system but is built into node.
+
+  Your Worker may throw errors at runtime unless you enable the "nodejs_compat"
+  compatibility flag. Refer to
+  https://developers.cloudflare.com/workers/runtime-apis/nodejs/ for more details.
+  Imported from:
+   - src/randomBytes.ts
+```
+
+However, in a lot of cases, it's possible to know at _build_ time whether the import is safe. This change also injects `navigator.userAgent` into `esbuild`'s bundle settings as a predefined constant, which means that `esbuild` can tree-shake away imports of `node:*` APIs that are guaranteed not to be hit at runtime, supressing the warning entirely.

packages/wrangler/src/__tests__/deploy.test.ts

@@ -7938,7 +7938,7 @@ export default{
 	});
 
 	describe("`nodejs_compat` compatibility flag", () => {
-		it('when absent, should error on any "external" `node:*` imports', async () => {
+		it('when absent, should warn on any "external" `node:*` imports', async () => {
 			writeWranglerToml();
 			fs.writeFileSync(
 				"index.js",
@@ -7948,15 +7948,18 @@ export default{
       export default {}
       `
 			);
-			let err: esbuild.BuildFailure | undefined;
-			try {
-				await runWrangler("deploy index.js --dry-run"); // expecting this to throw, as node compatibility isn't enabled
-			} catch (e) {
-				err = e as esbuild.BuildFailure;
-			}
-			expect(
-				esbuild.formatMessagesSync(err?.errors ?? [], { kind: "error" }).join()
-			).toMatch(/Could not resolve "node:async_hooks"/);
+			await runWrangler("deploy index.js --dry-run");
+
+			expect(std.warn).toMatchInlineSnapshot(`
+			"▲ [WARNING] The package \\"node:async_hooks\\" wasn't found on the file system but is built into node.
+
+			  Your Worker may throw errors at runtime unless you enable the \\"nodejs_compat\\" compatibility flag.
+			  Refer to https://developers.cloudflare.com/workers/runtime-apis/nodejs/ for more details. Imported
+			  from:
+			   - index.js
+
+			"
+		`);
 		});
 
 		it('when present, should support any "external" `node:*` imports', async () => {

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

@@ -0,0 +1,193 @@
+import assert from "node:assert";
+import { mkdir, readFile, writeFile } from "node:fs/promises";
+import path from "node:path";
+import dedent from "ts-dedent";
+import { bundleWorker } from "../deployment-bundle/bundle";
+import { noopModuleCollector } from "../deployment-bundle/module-collection";
+import { isNavigatorDefined } from "../navigator-user-agent";
+import { mockConsoleMethods } from "./helpers/mock-console";
+import { runInTempDir } from "./helpers/run-in-tmp";
+
+/*
+ * This file contains inline comments with the word "javascript"
+ * This signals to a compatible editor extension that the template string
+ * contents should be syntax-highlighted as JavaScript. One such extension
+ * is zjcompt.es6-string-javascript, but there are others.
+ */
+
+async function seedFs(files: Record<string, string>): Promise<void> {
+	for (const [location, contents] of Object.entries(files)) {
+		await mkdir(path.dirname(location), { recursive: true });
+		await writeFile(location, contents);
+	}
+}
+
+describe("isNavigatorDefined", () => {
+	test("default", () => {
+		expect(isNavigatorDefined(undefined)).toBe(false);
+	});
+
+	test("modern date", () => {
+		expect(isNavigatorDefined("2024-01-01")).toBe(true);
+	});
+
+	test("old date", () => {
+		expect(isNavigatorDefined("2000-01-01")).toBe(false);
+	});
+
+	test("switch date", () => {
+		expect(isNavigatorDefined("2022-03-21")).toBe(true);
+	});
+
+	test("before date", () => {
+		expect(isNavigatorDefined("2022-03-20")).toBe(false);
+	});
+
+	test("old date, but with flag", () => {
+		expect(isNavigatorDefined("2000-01-01", ["global_navigator"])).toBe(true);
+	});
+
+	test("old date, with disable flag", () => {
+		expect(isNavigatorDefined("2000-01-01", ["no_global_navigator"])).toBe(
+			false
+		);
+	});
+
+	test("new date, but with disable flag", () => {
+		expect(isNavigatorDefined("2024-01-01", ["no_global_navigator"])).toBe(
+			false
+		);
+	});
+
+	test("new date, with enable flag", () => {
+		expect(isNavigatorDefined("2024-01-01", ["global_navigator"])).toBe(true);
+	});
+
+	test("errors with disable and enable flags specified", () => {
+		try {
+			isNavigatorDefined("2024-01-01", [
+				"no_global_navigator",
+				"global_navigator",
+			]);
+			assert(false, "Unreachable");
+		} catch (e) {
+			expect(e).toMatchInlineSnapshot(
+				`[AssertionError: Can't both enable and disable a flag]`
+			);
+		}
+	});
+});
+
+// Does bundleWorker respect the value of `defineNavigatorUserAgent`?
+describe("defineNavigatorUserAgent is respected", () => {
+	runInTempDir();
+	const std = mockConsoleMethods();
+
+	it("defineNavigatorUserAgent = false, navigator preserved", async () => {
+		await seedFs({
+			"src/index.js": dedent/* javascript */ `
+			function randomBytes(length) {
+				if (navigator.userAgent !== "Cloudflare-Workers") {
+					return new Uint8Array(require("node:crypto").randomBytes(length));
+				} else {
+					return crypto.getRandomValues(new Uint8Array(length));
+				}
+			}
+			export default {
+				async fetch(request, env) {
+					return new Response(randomBytes(10))
+				},
+			};
+		`,
+		});
+
+		await bundleWorker(
+			{
+				file: path.resolve("src/index.js"),
+				directory: process.cwd(),
+				format: "modules",
+				moduleRoot: path.dirname(path.resolve("src/index.js")),
+			},
+			path.resolve("dist"),
+			{
+				bundle: true,
+				additionalModules: [],
+				moduleCollector: noopModuleCollector,
+				serveAssetsFromWorker: false,
+				doBindings: [],
+				define: {},
+				checkFetch: false,
+				targetConsumer: "deploy",
+				local: true,
+				projectRoot: process.cwd(),
+				defineNavigatorUserAgent: false,
+			}
+		);
+
+		// Build time warning that the dynamic import of `require("node:crypto")` may not be safe
+		expect(std.warn).toMatchInlineSnapshot(`
+		"▲ [WARNING] The package \\"node:crypto\\" wasn't found on the file system but is built into node.
+
+		  Your Worker may throw errors at runtime unless you enable the \\"nodejs_compat\\" compatibility flag.
+		  Refer to https://developers.cloudflare.com/workers/runtime-apis/nodejs/ for more details. Imported
+		  from:
+		   - src/index.js
+
+		"
+	`);
+		const fileContents = await readFile("dist/index.js", "utf8");
+
+		// navigator.userAgent should have been preserved as-is
+		expect(fileContents).toContain("navigator.userAgent");
+	});
+
+	it("defineNavigatorUserAgent = true, navigator treeshaken", async () => {
+		await seedFs({
+			"src/index.js": dedent/* javascript */ `
+			function randomBytes(length) {
+				if (navigator.userAgent !== "Cloudflare-Workers") {
+					return new Uint8Array(require("node:crypto").randomBytes(length));
+				} else {
+					return crypto.getRandomValues(new Uint8Array(length));
+				}
+			}
+			export default {
+				async fetch(request, env) {
+					return new Response(randomBytes(10))
+				},
+			};
+		`,
+		});
+
+		await bundleWorker(
+			{
+				file: path.resolve("src/index.js"),
+				directory: process.cwd(),
+				format: "modules",
+				moduleRoot: path.dirname(path.resolve("src/index.js")),
+			},
+			path.resolve("dist"),
+			{
+				bundle: true,
+				additionalModules: [],
+				moduleCollector: noopModuleCollector,
+				serveAssetsFromWorker: false,
+				doBindings: [],
+				define: {},
+				checkFetch: false,
+				targetConsumer: "deploy",
+				local: true,
+				projectRoot: process.cwd(),
+				defineNavigatorUserAgent: true,
+			}
+		);
+
+		// Build time warning is suppressed, because esbuild treeshakes the relevant code path
+		expect(std.warn).toMatchInlineSnapshot(`""`);
+
+		const fileContents = await readFile("dist/index.js", "utf8");
+
+		// navigator.userAgent should have been defined, and so should not be present in the bundle
+		expect(fileContents).not.toContain("navigator.userAgent");
+	});
+});

packages/wrangler/src/__tests__/pages/functions-build.test.ts

@@ -413,7 +413,7 @@ export default {
 		);
 	});
 
-	it("should error at Node.js imports when the `nodejs_compat` compatibility flag is not set", async () => {
+	it("should warn at Node.js imports when the `nodejs_compat` compatibility flag is not set", async () => {
 		mkdirSync("functions");
 		writeFileSync(
 			"functions/hello.js",
@@ -428,17 +428,18 @@ export default {
 		);
 
 		await expect(
-			runWrangler(`pages functions build --outfile=public/_worker.bundle`)
-		).rejects.toThrowErrorMatchingInlineSnapshot(`
-		"Build failed with 1 error:
-		hello.js:2:36: ERROR: Could not resolve \\"node:async_hooks\\""
-	`);
-		expect(std.err).toContain(
-			'The package "node:async_hooks" wasn\'t found on the file system but is built into node.'
-		);
-		expect(std.err).toContain(
-			'Add the "nodejs_compat" compatibility flag to your Pages project and make sure to prefix the module name with "node:" to enable Node.js compatibility.'
+			await runWrangler(`pages functions build --outfile=public/_worker.bundle`)
 		);
+		expect(std.warn).toMatchInlineSnapshot(`
+		"▲ [WARNING] The package \\"node:async_hooks\\" wasn't found on the file system but is built into node.
+
+		  Your Worker may throw errors at runtime unless you enable the \\"nodejs_compat\\" compatibility flag.
+		  Refer to https://developers.cloudflare.com/workers/runtime-apis/nodejs/ for more details. Imported
+		  from:
+		   - hello.js
+
+		"
+	`);
 	});
 
 	it("should compile a _worker.js/ directory", async () => {

packages/wrangler/src/api/pages/deploy.tsx

@@ -5,6 +5,7 @@ import { File, FormData } from "undici";
 import { fetchResult } from "../../cfetch";
 import { FatalError } from "../../errors";
 import { logger } from "../../logger";
+import { isNavigatorDefined } from "../../navigator-user-agent";
 import { buildFunctions } from "../../pages/buildFunctions";
 import { MAX_DEPLOYMENT_ATTEMPTS } from "../../pages/constants";
 import {
@@ -143,6 +144,10 @@ export async function deploy({
 	const nodejsCompat =
 		deploymentConfig.compatibility_flags?.includes("nodejs_compat");
 
+	const defineNavigatorUserAgent = isNavigatorDefined(
+		deploymentConfig.compatibility_date,
+		deploymentConfig.compatibility_flags
+	);
 	/**
 	 * Evaluate if this is an Advanced Mode or Pages Functions project. If Advanced Mode, we'll
 	 * go ahead and upload `_worker.js` as is, but if Pages Functions, we need to attempt to build
@@ -175,6 +180,7 @@ export async function deploy({
 				routesOutputPath,
 				local: false,
 				nodejsCompat,
+				defineNavigatorUserAgent,
 			});
 
 			builtFunctions = readFileSync(
@@ -254,6 +260,7 @@ export async function deploy({
 			workerJSDirectory: _workerPath,
 			buildOutputDirectory: directory,
 			nodejsCompat,
+			defineNavigatorUserAgent,
 		});
 	} else if (_workerJS) {
 		if (bundle) {
@@ -270,6 +277,7 @@ export async function deploy({
 				watch: false,
 				onEnd: () => {},
 				nodejsCompat,
+				defineNavigatorUserAgent,
 			});
 		} else {
 			await checkRawWorker(_workerPath, () => {});

packages/wrangler/src/deploy/deploy.ts

@@ -26,6 +26,7 @@
 import { UserError } from "../errors";
 import { logger } from "../logger";
 import { getMetricsUsageHeaders } from "../metrics";
+import { isNavigatorDefined } from "../navigator-user-agent";
 import { APIError, ParseError } from "../parse";
 import { getWranglerTmpDir } from "../paths";
 import { getQueue, putConsumer } from "../queues/client";
@@ -520,6 +521,10 @@
 							targetConsumer: "deploy",
 							local: false,
 							projectRoot: props.projectRoot,
+							defineNavigatorUserAgent: isNavigatorDefined(
+								props.compatibilityDate ?? config.compatibility_date,
+								props.compatibilityFlags ?? config.compatibility_flags
+							),
 						}
 				  );
 
@@ -536,6 +541,19 @@
 			dependencies[modulePath] = { bytesInOutput };
 		}
 
+		// Add modules to dependencies for size warning
+		for (const module of modules) {
+			const modulePath =
+				module.filePath === undefined
+					? module.name
+					: path.relative("", module.filePath);
+			const bytesInOutput =
+				typeof module.content === "string"
+					? Buffer.byteLength(module.content)
+					: module.content.byteLength;
+			dependencies[modulePath] = { bytesInOutput };
+		}
+
 		const content = readFileSync(resolvedEntryPointPath, {
 			encoding: "utf-8",
 		});