fix: render a helpful build error if a Service Worker mode Worker has imports

.changeset/perfect-experts-draw.md

@@ -0,0 +1,15 @@
+---
+"wrangler": patch
+---
+
+fix: render a helpful build error if a Service Worker mode Worker has imports
+
+A common mistake is to forget to export from the entry-point of a Worker, which causes
+Wrangler to infer that we are in "Service Worker" mode.
+
+In this mode, imports to external modules are not allowed.
+Currently this only fails at runtime, because our esbuild step converts these imports to an internal `__require()` call that throws an error.
+The error message is misleading and does not help the user identify the cause of the problem.
+This is particularly tricky where the external imports are added by a library or our own node.js polyfills.
+
+Fixes #6648

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

@@ -8851,6 +8851,125 @@ addEventListener('fetch', event => {});`
 		});
 	});
 
+	describe("service worker format", () => {
+		it("should error if trying to import a cloudflare prefixed external when in service worker format", async () => {
+			writeWranglerToml();
+			fs.writeFileSync(
+				"dep-1.js",
+				dedent`
+					import sockets from 'cloudflare:sockets';
+					export const external = sockets;
+				`
+			);
+			fs.writeFileSync(
+				"dep-2.js",
+				dedent`
+					export const internal = 100;
+				`
+			);
+			fs.writeFileSync(
+				"index.js",
+				dedent`
+					import {external} from "./dep-1"; // will the external import check be transitive?
+					import {internal} from "./dep-2"; // ensure that we can still have a non-external import
+					let x = [external, internal]; // to ensure that esbuild doesn't tree shake the imports
+					// no default export making this a service worker format
+					addEventListener('fetch', (event) => {
+						event.respondWith(new Response(''));
+					});
+			`
+			);
+
+			await expect(
+				runWrangler("deploy index.js --dry-run").catch(
+					(e) =>
+						normalizeString(
+							esbuild
+								.formatMessagesSync(e?.errors ?? [], { kind: "error" })
+								.join()
+								.trim()
+						).split("This error came from the")[0]
+				)
+			).resolves.toMatchInlineSnapshot(`
+				"X [ERROR] Unexpected external import of \\"cloudflare:sockets\\". Imports are not valid in a Service Worker format Worker.
+				Did you mean to create a Module Worker?
+				If so, try adding \`export default { ... }\` in your entry-point.
+				See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/. [plugin Cloudflare internal imports plugin]
+
+				  "
+			`);
+		});
+
+		it("should error if importing a node.js library when in service worker format", async () => {
+			writeWranglerToml();
+			fs.writeFileSync(
+				"index.js",
+				dedent`
+					import stream from "node:stream";
+					let temp = stream;
+					addEventListener('fetch', (event) => {
+						event.respondWith(new Response(''));
+					});
+			`
+			);
+
+			await expect(
+				runWrangler("deploy index.js --dry-run").catch(
+					(e) =>
+						normalizeString(
+							esbuild
+								.formatMessagesSync(e?.errors ?? [], { kind: "error" })
+								.join()
+								.trim()
+						).split("This error came from the")[0]
+				)
+			).resolves.toMatchInlineSnapshot(`
+				"X [ERROR]
+										Unexpected external import of \\"node:stream\\". Imports are not valid in a Service Worker format Worker.
+										Did you mean to create a Module Worker?
+										If so, try adding \`export default { ... }\` in your entry-point.
+										See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
+									 [plugin nodejs_compat imports plugin]
+
+				  "
+			`);
+		});
+
+		it("should error if nodejs_compat (v2) is turned on when in service worker format", async () => {
+			writeWranglerToml({
+				compatibility_date: "2024-09-23", // Sept 23 to turn on nodejs compat v2 mode
+				compatibility_flags: ["nodejs_compat"],
+			});
+			fs.writeFileSync(
+				"index.js",
+				dedent`
+					addEventListener('fetch', (event) => {
+						event.respondWith(new Response(''));
+					});
+			`
+			);
+
+			await expect(
+				runWrangler("deploy index.js --dry-run").catch(
+					(e) =>
+						normalizeString(
+							esbuild
+								.formatMessagesSync(e?.errors ?? [], { kind: "error" })
+								.join()
+								.trim()
+						).split("This error came from the")[0]
+				)
+			).resolves.toMatchInlineSnapshot(`
+				"X [ERROR] Unexpected external import of \\"node:stream\\" and \\"node:timers/promises\\". Imports are not valid in a Service Worker format Worker.
+				Did you mean to create a Module Worker?
+				If so, try adding \`export default { ... }\` in your entry-point.
+				See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/. [plugin unenv-cloudflare]
+
+				  "
+			`);
+		});
+	});
+
 	describe("legacy module specifiers", () => {
 		it("should work with legacy module specifiers, with a deprecation warning (1)", async () => {
 			writeWranglerToml({

packages/wrangler/src/deployment-bundle/esbuild-plugins/cloudflare-internal.ts

@@ -1,3 +1,4 @@
+import { dedent } from "../../utils/dedent";
 import type { Plugin } from "esbuild";
 
 /**
@@ -6,8 +7,29 @@ import type { Plugin } from "esbuild";
 export const cloudflareInternalPlugin: Plugin = {
 	name: "Cloudflare internal imports plugin",
 	setup(pluginBuild) {
-		pluginBuild.onResolve({ filter: /^cloudflare:.*/ }, () => {
+		const paths = new Set();
+		pluginBuild.onStart(() => paths.clear());
+		pluginBuild.onResolve({ filter: /^cloudflare:.*/ }, (args) => {
+			paths.add(args.path);
 			return { external: true };
 		});
+		pluginBuild.onEnd(() => {
+			if (pluginBuild.initialOptions.format === "iife" && paths.size > 0) {
+				// If we are bundling in "Service Worker" mode,
+				// imports of external modules such as `cloudflare:...`,
+				// which won't be inlined/bundled by esbuild, are invalid.
+				const pathList = new Intl.ListFormat("en-US").format(
+					Array.from(paths.keys()).map((p) => `"${p}"`)
+				);
+				throw new Error(
+					dedent`
+						Unexpected external import of ${pathList}. Imports are not valid in a Service Worker format Worker.
+						Did you mean to create a Module Worker?
+						If so, try adding \`export default { ... }\` in your entry-point.
+						See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
+					`
+				);
+			}
+		});
 	},
 };

packages/wrangler/src/deployment-bundle/esbuild-plugins/hybrid-nodejs-compat.ts

@@ -12,22 +12,52 @@ export const nodejsHybridPlugin: () => Plugin = () => {
 	return {
 		name: "unenv-cloudflare",
 		setup(build) {
+			errorOnServiceWorkerFormat(build);
 			handleRequireCallsToNodeJSBuiltins(build);
 			handleAliasedNodeJSPackages(build, alias, external);
 			handleNodeJSGlobals(build, inject);
 		},
 	};
 };
 
+const NODEJS_MODULES_RE = new RegExp(`^(node:)?(${builtinModules.join("|")})$`);
+
+/**
+ * If we are bundling a "Service Worker" formatted Worker, imports of external modules,
+ * which won't be inlined/bundled by esbuild, are invalid.
+ *
+ * This `onResolve()` handler will error if it identifies node.js external imports.
+ */
+function errorOnServiceWorkerFormat(build: PluginBuild) {
+	const paths = new Set();
+	build.onStart(() => paths.clear());
+	build.onResolve({ filter: NODEJS_MODULES_RE }, (args) => {
+		paths.add(args.path);
+		return null;
+	});
+	build.onEnd(() => {
+		if (build.initialOptions.format === "iife" && paths.size > 0) {
+			const pathList = new Intl.ListFormat("en-US").format(
+				Array.from(paths.keys()).map((p) => `"${p}"`)
+			);
+			throw new Error(
+				dedent`
+					Unexpected external import of ${pathList}. Imports are not valid in a Service Worker format Worker.
+					Did you mean to create a Module Worker?
+					If so, try adding \`export default { ... }\` in your entry-point.
+					See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
+				`
+			);
+		}
+	});
+}
+
 /**
  * We must convert `require()` calls for Node.js to a virtual ES Module that can be imported avoiding the require calls.
  * We do this by creating a special virtual ES module that re-exports the library in an onLoad handler.
  * The onLoad handler is triggered by matching the "namespace" added to the resolve.
  */
 function handleRequireCallsToNodeJSBuiltins(build: PluginBuild) {
-	const NODEJS_MODULES_RE = new RegExp(
-		`^(node:)?(${builtinModules.join("|")})$`
-	);
 	build.onResolve({ filter: NODEJS_MODULES_RE }, (args) => {
 		if (args.kind === "require-call") {
 			return {

packages/wrangler/src/deployment-bundle/esbuild-plugins/nodejs-compat.ts

@@ -1,6 +1,7 @@
 import { relative } from "path";
 import chalk from "chalk";
 import { logger } from "../../logger";
+import { dedent } from "../../utils/dedent";
 import type { Plugin } from "esbuild";
 
 /**
@@ -53,25 +54,57 @@ export const nodejsCompatPlugin: (silenceWarnings: boolean) => Plugin = (
 				return result;
 			}
 		);
+
+		/**
+		 * If we are bundling a "Service Worker" formatted Worker, imports of external modules,
+		 * which won't be inlined/bundled by esbuild, are invalid.
+		 *
+		 * This `onEnd()` handler will error if it identifies node.js external imports.
+		 */
+		pluginBuild.onEnd(() => {
+			if (
+				pluginBuild.initialOptions.format === "iife" &&
+				warnedPackaged.size > 0
+			) {
+				const paths = new Intl.ListFormat("en-US").format(
+					Array.from(warnedPackaged.keys()).map((p) => `"${p}"`)
+				);
+				throw new Error(`
+						Unexpected external import of ${paths}. Imports are not valid in a Service Worker format Worker.
+						Did you mean to create a Module Worker?
+						If so, try adding \`export default { ... }\` in your entry-point.
+						See https://developers.cloudflare.com/workers/reference/migrate-to-module-workers/.
+					`);
+				const errors = Array.from(warnedPackaged.entries()).map(
+					([path, importers]) =>
+						`Unexpected import "${path}" which is not valid in a Service Worker format Worker. Are you missing \`export default { ... }\` from your Worker?\n` +
+						"Imported from:\n" +
+						toList(importers, pluginBuild.initialOptions.absWorkingDir) +
+						"\n"
+				);
+				throw new Error(errors.join(""));
+			}
+		});
+
 		// Wait until the build finishes to log warnings, so that all files which import a package
 		// can be collated
 		pluginBuild.onEnd(() => {
 			if (!silenceWarnings) {
 				warnedPackaged.forEach((importers: string[], path: string) => {
 					logger.warn(
-						`The package "${path}" 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:
-${importers
-	.map(
-		(i) =>
-			` - ${chalk.blue(
-				relative(pluginBuild.initialOptions.absWorkingDir ?? "/", i)
-			)}`
-	)
-	.join("\n")}`
+						dedent`
+						The package "${path}" 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:
+						${toList(importers, pluginBuild.initialOptions.absWorkingDir)}`
 					);
 				});
 			}
 		});
 	},
 });
+
+function toList(items: string[], absWorkingDir: string | undefined): string {
+	return items
+		.map((i) => ` - ${chalk.blue(relative(absWorkingDir ?? "/", i))}`)
+		.join("\n");
+}