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 };