Avoid blocking built-in node modules when using --no-bundle flag (#4303)

* Avoid blocking built-in node modules when using --no-bundle flag * fixup! Avoid blocking built-in node modules when using --no-bundle flag * fixup! Avoid blocking built-in node modules when using --no-bundle flag --------- Co-authored-by: Peter Bacon Darwin <pbacondarwin@cloudflare.com>
cloudflare · Feb 20, 2024 · 1c46028 · 1c46028
1 parent 73b20fa
commit 1c46028
Show file tree

Hide file tree

Showing 5 changed files with 158 additions and 28 deletions.
diff --git a/.changeset/brave-years-wave.md b/.changeset/brave-years-wave.md
@@ -0,0 +1,5 @@
+---
+"wrangler": patch
+---
+
+Allow page functions to import builtin modules, even when not bundling with wrangler
diff --git a/packages/wrangler/src/__tests__/pages/deploy.test.ts b/packages/wrangler/src/__tests__/pages/deploy.test.ts
@@ -2868,7 +2868,8 @@ async function onRequest() {
 			contents.includes("worker_default as default");
 
 		const simulateServer = (
-			generatedWorkerBundleCheck: (workerJsContent: string) => void
+			generatedWorkerBundleCheck: (workerJsContent: string) => void,
+			compatibility_flags?: string[]
 		) => {
 			mockGetUploadTokenRequest(
 				"<<funfetti-auth-jwt>>",
@@ -2929,7 +2930,10 @@ async function onRequest() {
 								errors: [],
 								messages: [],
 								result: {
-									deployment_configs: { production: {}, preview: {} },
+									deployment_configs: {
+										production: { compatibility_flags },
+										preview: { compatibility_flags },
+									},
 								},
 							})
 						)
@@ -2953,6 +2957,108 @@ async function onRequest() {
 			expect(std.out).toContain("✨ Uploading Worker bundle");
 		});
 
+		it("should not allow 3rd party imports when not bundling", async () => {
+			// Add in a 3rd party import to the bundle
+			writeFileSync(
+				"public/_worker.js",
+				`
+				import { Test } from "test-package";
+
+				export default {
+					async fetch() {
+						console.log(Test);
+						return new Response("Ok");
+					},
+				};`
+			);
+
+			simulateServer((generatedWorkerJS) =>
+				expect(workerIsBundled(generatedWorkerJS)).toBeFalsy()
+			);
+			let error = "Code did not throw!";
+			try {
+				await runWrangler("pages deploy public --project-name=foo --no-bundle");
+			} catch (e) {
+				error = `${e}`;
+			}
+			expect(error).toContain(
+				"ERROR: [plugin: block-worker-js-imports] _worker.js is not being bundled by Wrangler but it is importing from another file."
+			);
+		});
+
+		it("should allow `cloudflare:...` imports when not bundling", async () => {
+			// Add in a 3rd party import to the bundle
+			writeFileSync(
+				"public/_worker.js",
+				`
+				import { EmailMessage } from "cloudflare:email";
+
+				export default {
+					async fetch() {
+						console.log("EmailMessage", EmailMessage);
+						return new Response("Ok");
+					},
+				};`
+			);
+
+			simulateServer((generatedWorkerJS) =>
+				expect(workerIsBundled(generatedWorkerJS)).toBeFalsy()
+			);
+			await runWrangler("pages deploy public --project-name=foo --no-bundle");
+			expect(std.out).toContain("✨ Uploading Worker bundle");
+		});
+
+		it("should allow `node:...` imports when not bundling and marked with nodejs_compat", async () => {
+			// Add in a node built-in import to the bundle
+			writeFileSync(
+				"public/_worker.js",
+				`
+				import { Buffer } from "node:buffer";
+
+				export default {
+					async fetch() {
+						return new Response(Buffer.from("Ok", "utf8"));
+					},
+				};`
+			);
+
+			simulateServer(
+				(generatedWorkerJS) =>
+					expect(workerIsBundled(generatedWorkerJS)).toBeFalsy(),
+				["nodejs_compat"]
+			);
+			await runWrangler("pages deploy public --project-name=foo --no-bundle");
+			expect(std.out).toContain("✨ Uploading Worker bundle");
+		});
+
+		it("should not allow `node:...` imports when not bundling and not marked nodejs_compat", async () => {
+			// Add in a node built-in import to the bundle
+			writeFileSync(
+				"public/_worker.js",
+				`
+				import { Buffer } from "node:buffer";
+
+				export default {
+					async fetch() {
+						return new Response(Buffer.from("Ok", "utf8"));
+					},
+				};`
+			);
+
+			simulateServer((generatedWorkerJS) =>
+				expect(workerIsBundled(generatedWorkerJS)).toBeFalsy()
+			);
+			let error = "Code did not throw!";
+			try {
+				await runWrangler("pages deploy public --project-name=foo --no-bundle");
+			} catch (e) {
+				error = `${e}`;
+			}
+			expect(error).toContain(
+				"ERROR: [plugin: block-worker-js-imports] _worker.js is not being bundled by Wrangler but it is importing from another file."
+			);
+		});
+
 		it("should not bundle the _worker.js when `--bundle` is set to false", async () => {
 			simulateServer((generatedWorkerJS) =>
 				expect(workerIsBundled(generatedWorkerJS)).toBeFalsy()

diff --git a/packages/wrangler/src/api/pages/deploy.tsx b/packages/wrangler/src/api/pages/deploy.tsx
@@ -142,7 +142,7 @@ export async function deploy({
 	const deploymentConfig =
 		project.deployment_configs[isProduction ? "production" : "preview"];
 	const nodejsCompat =
-		deploymentConfig.compatibility_flags?.includes("nodejs_compat");
+		deploymentConfig.compatibility_flags?.includes("nodejs_compat") ?? false;
 
 	const defineNavigatorUserAgent = isNavigatorDefined(
 		deploymentConfig.compatibility_date,
@@ -280,7 +280,7 @@ export async function deploy({
 				defineNavigatorUserAgent,
 			});
 		} else {
-			await checkRawWorker(_workerPath, () => {});
+			await checkRawWorker(_workerPath, nodejsCompat, () => {});
 			// TODO: Let users configure this in the future.
 			workerBundle = {
 				modules: [],

diff --git a/packages/wrangler/src/pages/dev.ts b/packages/wrangler/src/pages/dev.ts
@@ -316,7 +316,7 @@ export const Handler = async ({
 
 	let scriptPath = "";
 
-	const nodejsCompat = compatibilityFlags?.includes("nodejs_compat");
+	const nodejsCompat = compatibilityFlags?.includes("nodejs_compat") ?? false;
 	const defineNavigatorUserAgent = isNavigatorDefined(
 		compatibilityDate,
 		compatibilityFlags
@@ -354,7 +354,9 @@ export const Handler = async ({
 	} else if (usingWorkerScript) {
 		scriptPath = workerScriptPath;
 		let runBuild = async () => {
-			await checkRawWorker(workerScriptPath, () => scriptReadyResolve());
+			await checkRawWorker(workerScriptPath, nodejsCompat, () =>
+				scriptReadyResolve()
+			);
 		};
 
 		if (enableBundling) {

diff --git a/packages/wrangler/src/pages/functions/buildWorker.ts b/packages/wrangler/src/pages/functions/buildWorker.ts
@@ -351,33 +351,50 @@ export function buildNotifierPlugin(onEnd: () => void): Plugin {
  * This is useful when the user chooses not to bundle the `_worker.js` file by setting
  * `--no-bundle` at the command line.
  */
-export async function checkRawWorker(scriptPath: string, onEnd: () => void) {
+export async function checkRawWorker(
+	scriptPath: string,
+	nodejsCompat: boolean,
+	onEnd: () => void
+) {
 	await esBuild({
 		entryPoints: [scriptPath],
 		write: false,
 		// we need it to be bundled so that any imports that are used are affected by the blocker plugin
 		bundle: true,
-		plugins: [blockWorkerJsImports, buildNotifierPlugin(onEnd)],
+		plugins: [blockWorkerJsImports(nodejsCompat), buildNotifierPlugin(onEnd)],
+		logLevel: "silent",
 	});
 }
 
-const blockWorkerJsImports: Plugin = {
-	name: "block-worker-js-imports",
-	setup(build) {
-		build.onResolve({ filter: /.*/g }, (args) => {
-			// If it's the entrypoint, let it be as is
-			if (args.kind === "entry-point") {
-				return {
-					path: args.path,
-				};
-			}
-			// Otherwise, block any imports that the file is requesting
-			throw new FatalError(
-				"_worker.js is not being bundled by Wrangler but it is importing from another file.\n" +
-					"This will throw an error if deployed.\n" +
-					"You should bundle the Worker in a pre-build step, remove the import if it is unused, or ask Wrangler to bundle it by setting `--bundle`.",
-				1
-			);
-		});
-	},
-};
+function blockWorkerJsImports(nodejsCompat: boolean): Plugin {
+	return {
+		name: "block-worker-js-imports",
+		setup(build) {
+			build.onResolve({ filter: /.*/g }, (args) => {
+				// If it's the entrypoint, let it be as is
+				if (args.kind === "entry-point") {
+					return {
+						path: args.path,
+					};
+				}
+				// If it's a node or cf built-in, mark it as external
+				if (
+					(nodejsCompat && args.path.startsWith("node:")) ||
+					args.path.startsWith("cloudflare:")
+				) {
+					return {
+						path: args.path,
+						external: true,
+					};
+				}
+				// Otherwise, block any other imports that the file is requesting
+				throw new FatalError(
+					"_worker.js is not being bundled by Wrangler but it is importing from another file.\n" +
+						"This will throw an error if deployed.\n" +
+						"You should bundle the Worker in a pre-build step, remove the import if it is unused, or ask Wrangler to bundle it by setting `--bundle`.",
+					1
+				);
+			});
+		},
+	};
+}