Support no_bundle config in Pages for both dev and deploy. (#8500)

This was already supported via a command line arg (`--no-bundle`).

Co-authored-by: Peter Bacon Darwin <pbacondarwin@cloudflare.com>

Author: workers-devprod

.changeset/crazy-sheep-give.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+Support `no_bundle` config in Pages for both `dev` and `deploy`.
+
+This was already supported via a command line arg (`--no-bundle`).

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

@@ -1,5 +1,6 @@
 import { mkdirSync, writeFileSync } from "node:fs";
 import { chdir } from "node:process";
+import TOML from "@iarna/toml";
 import { execa } from "execa";
 import { http, HttpResponse } from "msw";
 import dedent from "ts-dedent";
@@ -74,7 +75,7 @@ describe("pages deploy", () => {
 			      --commit-message      The commit message to attach to this deployment  [string]
 			      --commit-dirty        Whether or not the workspace should be considered dirty for this deployment  [boolean]
 			      --skip-caching        Skip asset caching which speeds up builds  [boolean]
-			      --no-bundle           Whether to run bundling on \`_worker.js\` before deploying  [boolean] [default: false]
+			      --no-bundle           Whether to run bundling on \`_worker.js\` before deploying  [boolean]
 			      --upload-source-maps  Whether to upload any server-side sourcemaps with this deployment  [boolean] [default: false]"
 		`);
 	});
@@ -5425,6 +5426,51 @@ Failed to publish your Function. Got error: Uncaught TypeError: a is not a funct
 		});
 	});
 
+	describe("_worker.js directory bundling", () => {
+		const workerIsBundled = async (contents: FormDataEntryValue | null) =>
+			(await toString(contents)).includes("worker_default as default");
+
+		["wrangler.json", "wrangler.toml"].forEach((configPath) => {
+			it(
+				"should not bundle the _worker.js when `no_bundle = true` in Wrangler config: " +
+					configPath,
+				async () => {
+					mkdirSync("public/_worker.js", { recursive: true });
+					writeFileSync(
+						"public/_worker.js/index.js",
+						`
+					export default {
+						async fetch(request, env) {
+							return new Response('Ok');
+						}
+					};
+					`
+					);
+
+					const config = {
+						name: "foo",
+						no_bundle: true,
+						pages_build_output_dir: "public",
+					};
+					writeFileSync(
+						`${configPath}`,
+						configPath === "wrangler.json"
+							? JSON.stringify(config)
+							: TOML.stringify(config)
+					);
+
+					simulateServer((generatedWorkerJS) =>
+						expect(workerIsBundled(generatedWorkerJS)).resolves.toBeFalsy()
+					);
+
+					await runWrangler("pages deploy");
+
+					expect(std.out).toContain("✨ Uploading Worker bundle");
+				}
+			);
+		});
+	});
+
 	describe("source maps", () => {
 		const bundleString = (entry: FormDataEntryValue | null) =>
 			toString(entry).then((str) =>

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

@@ -63,7 +63,7 @@ describe("pages", () => {
 			      --inspector-port                             Port for devtools to connect to  [number]
 			      --proxy                                      The port to proxy (where the static assets are served)  [deprecated] [number]
 			      --script-path                                The location of the single Worker script if not using functions  [default: _worker.js]  [deprecated] [string]
-			      --no-bundle                                  Whether to run bundling on \`_worker.js\`  [boolean] [default: false]
+			      --no-bundle                                  Whether to run bundling on \`_worker.js\`  [boolean]
 			  -b, --binding                                    Bind variable/secret (KEY=VALUE)  [array]
 			  -k, --kv                                         KV namespace to bind (--kv KV_BINDING)  [array]
 			      --d1                                         D1 database to bind (--d1 D1_BINDING)  [array]
@@ -150,7 +150,7 @@ describe("pages", () => {
 			      --commit-message      The commit message to attach to this deployment  [string]
 			      --commit-dirty        Whether or not the workspace should be considered dirty for this deployment  [boolean]
 			      --skip-caching        Skip asset caching which speeds up builds  [boolean]
-			      --no-bundle           Whether to run bundling on \`_worker.js\` before deploying  [boolean] [default: false]
+			      --no-bundle           Whether to run bundling on \`_worker.js\` before deploying  [boolean]
 			      --upload-source-maps  Whether to upload any server-side sourcemaps with this deployment  [boolean] [default: false]"
 		`);
 	});

packages/wrangler/src/config/validation-pages.ts

@@ -19,6 +19,7 @@ const supportedPagesConfigFields = [
 	"compatibility_date",
 	"compatibility_flags",
 	"send_metrics",
+	"no_bundle",
 	"limits",
 	"placement",
 	"vars",

packages/wrangler/src/pages/deploy.ts

@@ -77,7 +77,8 @@ export function Options(yargs: CommonYargsArgv) {
 			},
 			"no-bundle": {
 				type: "boolean",
-				default: false,
+				default: undefined,
+				conflicts: "bundle",
 				description: "Whether to run bundling on `_worker.js` before deploying",
 			},
 			config: {
@@ -349,6 +350,8 @@ export const Handler = async (args: PagesDeployArgs) => {
 		}
 	}
 
+	const enableBundling = args.bundle ?? !(args.noBundle ?? config?.no_bundle);
+
 	const { deploymentResponse, formData } = await deploy({
 		directory,
 		accountId,
@@ -358,9 +361,7 @@ export const Handler = async (args: PagesDeployArgs) => {
 		commitHash,
 		commitDirty,
 		skipCaching: args.skipCaching,
-		// TODO: Here lies a known bug. If you specify both `--bundle` and `--no-bundle`, this behavior is undefined and you will get unexpected results.
-		// There is no sane way to get the true value out of yargs, so here we are.
-		bundle: args.bundle ?? !args.noBundle,
+		bundle: enableBundling,
 		// Sourcemaps from deploy arguments will take precedence so people can try it for one-off deployments without updating their wrangler.toml
 		sourceMaps: config?.upload_source_maps || args.uploadSourceMaps,
 		args,

packages/wrangler/src/pages/dev.ts

@@ -147,7 +147,8 @@ export function Options(yargs: CommonYargsArgv) {
 			},
 			"no-bundle": {
 				type: "boolean",
-				default: false,
+				default: undefined,
+				conflicts: "bundle",
 				description: "Whether to run bundling on `_worker.js`",
 			},
 			binding: {
@@ -357,9 +358,7 @@ export const Handler = async (args: PagesDevArguments) => {
 	const usingWorkerDirectory =
 		existsSync(workerScriptPath) && lstatSync(workerScriptPath).isDirectory();
 	const usingWorkerScript = existsSync(workerScriptPath);
-	// TODO: Here lies a known bug. If you specify both `--bundle` and `--no-bundle`, this behavior is undefined and you will get unexpected results.
-	// There is no sane way to get the true value out of yargs, so here we are.
-	const enableBundling = args.bundle ?? !args.noBundle;
+	const enableBundling = args.bundle ?? !(args.noBundle ?? config.no_bundle);
 
 	const functionsDirectory = "./functions";
 	let usingFunctions = !usingWorkerScript && existsSync(functionsDirectory);
@@ -371,7 +370,7 @@ export const Handler = async (args: PagesDevArguments) => {
 		args.compatibilityFlags ?? config.compatibility_flags ?? [],
 		{
 			nodeCompat: args.nodeCompat,
-			noBundle: args.noBundle ?? config.no_bundle,
+			noBundle: !enableBundling,
 		}
 	);