[wrangler] Fix named-only module Worker format detection

.changeset/bright-workers-listen.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+Detect named-only module Worker entrypoints correctly
+
+Wrangler now distinguishes named-only module Workers from legacy Service Workers that happen to have named exports. A default export identifies a module Worker; otherwise, legacy `addEventListener` registration identifies Service Worker format.

packages/wrangler/src/__tests__/guess-worker-format.test.ts

@@ -81,21 +81,276 @@ describe("guess worker format", () => {
 		expect(guess.format).toBe("service-worker");
 	});
 
-	it("logs a warning when a worker has exports, but not a default one", async ({
+	it.for([
+		{
+			name: "global scope",
+			source: `addEventListener("fetch", (event) => {
+				event.respondWith(new Response(foo.toString()));
+			});`,
+		},
+		{
+			name: "globalThis",
+			source: `globalThis.addEventListener("fetch", (event) => {
+				event.respondWith(new Response(foo.toString()));
+			});`,
+		},
+		{
+			name: "self",
+			source: `self.addEventListener("fetch", (event) => {
+				event.respondWith(new Response(foo.toString()));
+			});`,
+		},
+		{
+			name: "static bracket notation",
+			source: `self["addEventListener"]("fetch", (event) => {
+				event.respondWith(new Response(foo.toString()));
+			});`,
+		},
+		{
+			name: "a method alias",
+			source: `const register = self.addEventListener;
+			register.call(self, "fetch", (event) => {
+				event.respondWith(new Response(foo.toString()));
+			});`,
+		},
+	])(
+		"detects a legacy Service Worker with a named export using $name",
+		async ({ source }, { expect }) => {
+			await writeFile(
+				"./index.ts",
+				`
+				export const foo = 1;
+
+				${source}
+				`
+			);
+			const guess = await guessWorkerFormat(
+				path.join(process.cwd(), "./index.ts"),
+				process.cwd(),
+				undefined
+			);
+			expect(guess.format).toBe("service-worker");
+			expect(std.warn).toMatchInlineSnapshot(`
+				"▲ [WARNING] The entrypoint index.ts has exports like an ES Module, but hasn't defined a default export like a module worker normally would. Building the worker using "service-worker" format...
+
+				"
+			`);
+		}
+	);
+
+	it.for([
+		{
+			name: "locally shadowed globals",
+			source: `
+				function registerWithSelf(self: { addEventListener: (...args: unknown[]) => void }) {
+					self.addEventListener("fetch", () => {});
+				}
+				function registerWithGlobalThis(globalThis: { addEventListener: (...args: unknown[]) => void }) {
+					globalThis.addEventListener("fetch", () => {});
+				}
+				registerWithSelf({ addEventListener() {} });
+				registerWithGlobalThis({ addEventListener() {} });
+			`,
+		},
+		{
+			name: "receiver aliases",
+			source: `
+				const receiver = self;
+				receiver.addEventListener("fetch", (event) => {
+					event.respondWith(new Response(foo.toString()));
+				});
+			`,
+		},
+	])(
+		"chooses a Module Worker with a named export when using $name",
+		async ({ source }, { expect }) => {
+			await writeFile("./index.ts", `export const foo = 1;\n${source}`);
+			const guess = await guessWorkerFormat(
+				path.join(process.cwd(), "./index.ts"),
+				process.cwd(),
+				undefined
+			);
+			expect(guess.format).toBe("modules");
+		}
+	);
+
+	it("detects a Module Worker with only a named WorkerEntrypoint", async ({
 		expect,
 	}) => {
-		await writeFile("./index.ts", "export const foo = 1;");
+		await writeFile(
+			"./index.ts",
+			`
+			import { WorkerEntrypoint } from "cloudflare:workers";
+
+			export class NamedEntrypoint extends WorkerEntrypoint {
+				fetch(): Response {
+					return new Response("Hello from the named entrypoint");
+				}
+			}
+			`
+		);
 		const guess = await guessWorkerFormat(
 			path.join(process.cwd(), "./index.ts"),
 			process.cwd(),
 			undefined
 		);
-		expect(guess.format).toBe("service-worker");
-		expect(std.warn).toMatchInlineSnapshot(`
-			"▲ [WARNING] The entrypoint index.ts has exports like an ES Module, but hasn't defined a default export like a module worker normally would. Building the worker using "service-worker" format...
+		expect(guess.format).toBe("modules");
+		expect(std.warn).not.toContain(
+			'Building the worker using "service-worker" format'
+		);
+	});
+
+	it("detects a Module Worker with only a DurableObject", async ({
+		expect,
+	}) => {
+		await writeFile(
+			"./index.ts",
+			`
+			import { DurableObject } from "cloudflare:workers";
 
-			"
-		`);
+			export class NamedEntrypoint extends DurableObject {
+				fetch(): Response {
+					return new Response("Hello from the named entrypoint");
+				}
+			}
+			`
+		);
+		const guess = await guessWorkerFormat(
+			path.join(process.cwd(), "./index.ts"),
+			process.cwd(),
+			undefined
+		);
+		expect(guess.format).toBe("modules");
+		expect(std.warn).not.toContain(
+			'Building the worker using "service-worker" format'
+		);
+	});
+
+	it("detects a Module Worker with only a legacy DurableObject", async ({
+		expect,
+	}) => {
+		await writeFile(
+			"./index.ts",
+			`
+			export class SomeClass {
+				constructor(controller, env) {}
+
+				async fetch(request) {
+					return new Response("Actor!");
+				}
+			}
+			`
+		);
+		const guess = await guessWorkerFormat(
+			path.join(process.cwd(), "./index.ts"),
+			process.cwd(),
+			undefined
+		);
+		expect(guess.format).toBe("modules");
+		expect(std.warn).not.toContain(
+			'Building the worker using "service-worker" format'
+		);
+	});
+
+	it("detects Module Worker via default export over named WorkerEntrypoint and addEventListener", async ({
+		expect,
+	}) => {
+		await writeFile(
+			"./index.ts",
+			`
+			import { WorkerEntrypoint } from "cloudflare:workers";
+
+			export default {
+				fetch(): Response {
+					return new Response("Hello from the default entrypoint");
+				},
+			};
+
+			export class NamedEntrypoint extends WorkerEntrypoint {
+				fetch(): Response {
+					return new Response("Hello from the named entrypoint");
+				}
+			}
+
+			addEventListener("fetch", () => {});
+			`
+		);
+		const guess = await guessWorkerFormat(
+			path.join(process.cwd(), "./index.ts"),
+			process.cwd(),
+			undefined
+		);
+		expect(guess).toStrictEqual({
+			format: "modules",
+			exports: ["NamedEntrypoint", "default"],
+		});
+		expect(std.warn).not.toContain(
+			'Building the worker using "service-worker" format'
+		);
+	});
+
+	it("detects Service Worker format when a named Object and addEventListener are both present", async ({
+		expect,
+	}) => {
+		await writeFile(
+			"./index.ts",
+			`
+			export const NamedEntrypoint = {
+				fetch(): Response {
+					return new Response("Hello from the named entrypoint");
+				}
+			}
+
+			addEventListener("fetch", () => {});
+			`
+		);
+		const guess = await guessWorkerFormat(
+			path.join(process.cwd(), "./index.ts"),
+			process.cwd(),
+			undefined
+		);
+		expect(guess).toStrictEqual({
+			format: "service-worker",
+			exports: ["NamedEntrypoint"],
+		});
+		expect(std.warn).toContain(
+			'Building the worker using "service-worker" format'
+		);
+	});
+
+	// NOTE: this is very strange behavior, but it is intentional
+	// For backwards compatibility, the heuristic must assume SW syntax here,
+	// even though the file is guaranteed to fail the actual build later, because SW
+	// cannot perform internal imports.
+	it("detects Service Worker format when a named Entrypoint and addEventListener are both present", async ({
+		expect,
+	}) => {
+		await writeFile(
+			"./index.ts",
+			`
+			import { WorkerEntrypoint } from "cloudflare:workers";
+
+			export class NamedEntrypoint extends WorkerEntrypoint {
+				fetch(): Response {
+					return new Response("Hello from the named entrypoint");
+				}
+			}
+
+			addEventListener("fetch", () => {});
+			`
+		);
+		const guess = await guessWorkerFormat(
+			path.join(process.cwd(), "./index.ts"),
+			process.cwd(),
+			undefined
+		);
+		expect(guess).toStrictEqual({
+			format: "service-worker",
+			exports: ["NamedEntrypoint"],
+		});
+		expect(std.warn).toContain(
+			'Building the worker using "service-worker" format'
+		);
 	});
 
 	it("should list exports", async ({ expect }) => {

packages/wrangler/src/deployment-bundle/guess-worker-format.ts

@@ -5,13 +5,22 @@ import { COMMON_ESBUILD_OPTIONS } from "./bundle";
 import { getEntryPointFromMetafile } from "./entry-point-from-metafile";
 import type { CfScriptFormat } from "@cloudflare/workers-utils";
 
+const SERVICE_WORKER_EVENT_LISTENER =
+	"__WRANGLER_SERVICE_WORKER_EVENT_LISTENER__";
+
 /**
- * A function to "guess" the type of worker.
- * We do this by running a lightweight build of the actual script,
- * and looking at the meta-file generated by esbuild. If it has a default
- * export (or really, any exports), that means it's a "modules" worker.
- * Else, it's a "service-worker" worker. This seems hacky, but works remarkably
- * well in practice.
+ * Guesses the Worker format by running a lightweight build and inspecting its
+ * generated exports. For JavaScript and TypeScript entrypoints, the heuristic
+ * is:
+ *
+ * - No exports: Service Worker, regardless of event listener syntax.
+ * - A default export: Module Worker.
+ * - Named-only exports are ambiguous:
+ *   - A recognized global `addEventListener` reference: Service Worker.
+ *   - No recognized global `addEventListener` reference: Module Worker.
+ *
+ * An `addEventListener` reference does not necessarily need to be a call that
+ * registers an event listener.
  */
 export async function guessWorkerFormat(
 	entryFile: string,
@@ -23,6 +32,9 @@ export async function guessWorkerFormat(
 		return { format: "modules", exports: [] };
 	}
 
+	// Let esbuild mark references to the global binding so strings, comments, and
+	// locally shadowed functions named `addEventListener` aren't false positives.
+	// The marker identifies references, without determining how they are used.
 	const result = await esbuild.build({
 		...COMMON_ESBUILD_OPTIONS,
 		entryPoints: [entryFile],
@@ -31,28 +43,37 @@ export async function guessWorkerFormat(
 		bundle: false,
 		write: false,
 		...(tsconfig && { tsconfig }),
+		define: {
+			addEventListener: SERVICE_WORKER_EVENT_LISTENER,
+			"globalThis.addEventListener": SERVICE_WORKER_EVENT_LISTENER,
+			"self.addEventListener": SERVICE_WORKER_EVENT_LISTENER,
+		},
 		logLevel: "silent",
 	});
 
 	// result.metafile is defined because of the `metafile: true` option above.
 	const metafile = result.metafile;
 
 	const { exports } = getEntryPointFromMetafile(entryFile, metafile);
+	const usesServiceWorkerEventListener = result.outputFiles.some(({ text }) =>
+		text.includes(SERVICE_WORKER_EVENT_LISTENER)
+	);
+
 	let guessedWorkerFormat: CfScriptFormat;
-	if (exports.length > 0) {
-		if (exports.includes("default")) {
-			guessedWorkerFormat = "modules";
-		} else {
-			logger.warn(
-				`The entrypoint ${path.relative(
-					process.cwd(),
-					entryFile
-				)} has exports like an ES Module, but hasn't defined a default export like a module worker normally would. Building the worker using "service-worker" format...`
-			);
-			guessedWorkerFormat = "service-worker";
-		}
-	} else {
+	if (exports.length === 0) {
 		guessedWorkerFormat = "service-worker";
+	} else if (exports.includes("default")) {
+		guessedWorkerFormat = "modules";
+	} else if (usesServiceWorkerEventListener) {
+		logger.warn(
+			`The entrypoint ${path.relative(
+				process.cwd(),
+				entryFile
+			)} has exports like an ES Module, but hasn't defined a default export like a module worker normally would. Building the worker using "service-worker" format...`
+		);
+		guessedWorkerFormat = "service-worker";
+	} else {
+		guessedWorkerFormat = "modules";
 	}
 
 	return { format: guessedWorkerFormat, exports };