From 84f7e87e3c544e7c89290e6205df3db44074a099 Mon Sep 17 00:00:00 2001 From: Taylor Lee Date: Fri, 4 Sep 2026 17:51:34 -0700 Subject: [PATCH] [wrangler] Fix named-only module Worker format detection Fixes #15309. The legacy heuristic of assuming anything without a default export is going more harm than good. It also seems like it must have regressed at some point, because it used to be possible to have DO-class-only Workers a [few years ago](https://github.com/cloudflare/cloudflare-docs/pull/5185), even though trying that now would fail by detecting SW syntax. After looking at a bunch of options and various forms of required back-compat, I think the best path forward is to resolve the ambigious case in favor of modules workers. Current heuristic: * default export -> module worker * otherwise -> service worker New heuristic: * no exports -> service worker * default export -> module worker * ambigious exports: - recognized global addEventListener reference -> service worker - otherwise -> module worker This does that the most tricky dynamic constructions of SW can be falsely interpreted as module workers if they have some named exports. I think that's acceptable for a 2 reasons: 1. it is less harmful than misclassifying MW as SW 2. many of the most dynamic constructions (of either MW or SW) can't actually be deployed in practice (due to preexisting validator limitations), so the real breakage is minimal. Alternatives considered: export-only heuristics, source regexes, entrypoint-specific AST matching, an additional parser, and full dependency bundling. These were avoided due to compatibility risks, false positives, added complexity, or resolution overhead. --- .changeset/bright-workers-listen.md | 7 + .../src/__tests__/guess-worker-format.test.ts | 269 +++++++++++++++++- .../deployment-bundle/guess-worker-format.ts | 59 ++-- 3 files changed, 309 insertions(+), 26 deletions(-) create mode 100644 .changeset/bright-workers-listen.md diff --git a/.changeset/bright-workers-listen.md b/.changeset/bright-workers-listen.md new file mode 100644 index 00000000000..cf821f11e14 --- /dev/null +++ b/.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. diff --git a/packages/wrangler/src/__tests__/guess-worker-format.test.ts b/packages/wrangler/src/__tests__/guess-worker-format.test.ts index 230213f2689..dde08931607 100644 --- a/packages/wrangler/src/__tests__/guess-worker-format.test.ts +++ b/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 }) => { diff --git a/packages/wrangler/src/deployment-bundle/guess-worker-format.ts b/packages/wrangler/src/deployment-bundle/guess-worker-format.ts index 9f0d3a55be6..d674c3f30de 100644 --- a/packages/wrangler/src/deployment-bundle/guess-worker-format.ts +++ b/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,6 +43,11 @@ 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", }); @@ -38,21 +55,25 @@ export async function guessWorkerFormat( 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 };