fix: avoid injecting esbuild watch stubs into production Worker code (#…

…5960)

When we added the ability to include additional modules in the deployed bundle of a Worker,
we inadvertently also included some boiler plate code that is only needed at development time.

This fix ensures that this code is only injected if we are running esbuild in watch mode
(e.g. `wrangler dev`) and not when building for deployment.

It is interesting to note that this boilerplate only gets included in the production code
if there is an import of CommonJS code in the Worker, which esbuild needs to convert to an
ESM import.

Fixes [#4269](#4269)

.changeset/popular-ravens-add.md

@@ -0,0 +1,17 @@
+---
+"wrangler": patch
+---
+
+fix: avoid injecting esbuild watch stubs into production Worker code
+
+When we added the ability to include additional modules in the deployed bundle of a Worker,
+we inadvertently also included some boiler plate code that is only needed at development time.
+
+This fix ensures that this code is only injected if we are running esbuild in watch mode
+(e.g. `wrangler dev`) and not when building for deployment.
+
+It is interesting to note that this boilerplate only gets included in the production code
+if there is an import of CommonJS code in the Worker, which esbuild needs to convert to an
+ESM import.
+
+Fixes [#4269](https://github.com/cloudflare/workers-sdk/issues/4269)

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

@@ -2601,7 +2601,18 @@ addEventListener('fetch', event => {});`
 				find_additional_modules: true,
 				rules: [{ type: "ESModule", globs: ["**/*.mjs"] }],
 			});
-			writeWorkerSource({ type: "esm" });
+			// Create a Worker that imports a CommonJS module to trigger esbuild to add
+			// extra boilerplate to convert to ESM imports.
+			fs.writeFileSync(`another.cjs`, `module.exports.foo = 100;`);
+			fs.writeFileSync(
+				`index.js`,
+				`import { foo } from "./another.cjs";
+					export default {
+						async fetch(request) {
+							return new Response('Hello' + foo);
+						},
+					};`
+			);
 			fs.mkdirSync("a/b/c", { recursive: true });
 			fs.writeFileSync(
 				"a/1.mjs",
@@ -2621,6 +2632,11 @@ addEventListener('fetch', event => {});`
 			);
 			writeAssets(assets);
 			mockUploadWorkerRequest({
+				expectedEntry(entry) {
+					// Ensure that we have not included the watch stub in production code.
+					// This is only needed in `wrangler dev`.
+					expect(entry).not.toMatch(/modules-watch-stub\.js/);
+				},
 				expectedBindings: [
 					{
 						name: "__STATIC_CONTENT",

packages/wrangler/src/__tests__/helpers/mock-upload-worker.ts

@@ -9,7 +9,7 @@ import type { HttpResponseResolver } from "msw";
 export function mockUploadWorkerRequest(
 	options: {
 		available_on_subdomain?: boolean;
-		expectedEntry?: string | RegExp;
+		expectedEntry?: string | RegExp | ((entry: string | null) => void);
 		expectedMainModule?: string;
 		expectedType?: "esm" | "sw";
 		expectedBindings?: unknown;
@@ -47,8 +47,10 @@ export function mockUploadWorkerRequest(
 		}
 
 		const formBody = await request.formData();
-		if (expectedEntry !== undefined) {
+		if (typeof expectedEntry === "string" || expectedEntry instanceof RegExp) {
 			expect(await serialize(formBody.get("index.js"))).toMatch(expectedEntry);
+		} else if (typeof expectedEntry === "function") {
+			expectedEntry(await serialize(formBody.get("index.js")));
 		}
 		const metadata = JSON.parse(
 			await toString(formBody.get("metadata"))

packages/wrangler/src/deployment-bundle/bundle.ts

@@ -266,10 +266,12 @@ export async function bundleWorker(
 		inject.push(...(result.inject ?? []));
 	}
 
-	// `esbuild` doesn't support returning `watch*` options from `onStart()`
-	// plugin callbacks. Instead, we define an empty virtual module that is
-	// imported in this injected module. Importing that module registers watchers.
-	inject.push(path.resolve(getBasePath(), "templates/modules-watch-stub.js"));
+	if (watch) {
+		// `esbuild` doesn't support returning `watch*` options from `onStart()`
+		// plugin callbacks. Instead, we define an empty virtual module that is
+		// imported in this injected module. Importing that module registers watchers.
+		inject.push(path.resolve(getBasePath(), "templates/modules-watch-stub.js"));
+	}
 
 	const buildOptions: esbuild.BuildOptions & { metafile: true } = {
 		// Don't use entryFile here as the file may have been changed when applying the middleware