fix: only show fetch warning if on old compatibility_date (#7038)

* fix: only show fetch warning if on old compatibility_date

Now that we have the `allow_custom_ports` compatibility flag, we only need to show the fetch warnings when that flag is not enabled.

Fixes cloudflare/workerd#2955

* fix: typo

.changeset/shaggy-monkeys-visit.md

@@ -0,0 +1,9 @@
+---
+"wrangler": patch
+---
+
+fix: only show fetch warning if on old compatibility_date
+
+Now that we have the `allow_custom_ports` compatibility flag, we only need to show the fetch warnings when that flag is not enabled.
+
+Fixes https://github.com/cloudflare/workerd/issues/2955

packages/wrangler/src/__tests__/check-fetch.test.ts

@@ -0,0 +1,28 @@
+import "vitest";
+import { shouldCheckFetch } from "../deployment-bundle/bundle";
+
+describe("shouldCheckFetch()", () => {
+	it("should be true for old compat date", () => {
+		expect(shouldCheckFetch("2024-09-01")).toBe(true);
+	});
+
+	it("should be false for new compat date", () => {
+		expect(shouldCheckFetch("2024-09-02")).toBe(false);
+	});
+
+	it("should be true for old compat date + old compat flag", () => {
+		expect(shouldCheckFetch("2024-09-01", ["ignore_custom_ports"])).toBe(true);
+	});
+
+	it("should be false for old compat date + new compat flag", () => {
+		expect(shouldCheckFetch("2024-09-01", ["allow_custom_ports"])).toBe(false);
+	});
+
+	it("should be true for new compat date + old compat flag", () => {
+		expect(shouldCheckFetch("2024-09-02", ["ignore_custom_ports"])).toBe(true);
+	});
+
+	it("should be false for new compat date + new compat flag", () => {
+		expect(shouldCheckFetch("2024-09-02", ["allow_custom_ports"])).toBe(false);
+	});
+});

packages/wrangler/src/api/pages/deploy.ts

@@ -6,6 +6,7 @@ import { cwd } from "node:process";
 import { File, FormData } from "undici";
 import { fetchResult } from "../../cfetch";
 import { readConfig } from "../../config";
+import { shouldCheckFetch } from "../../deployment-bundle/bundle";
 import { validateNodeCompatMode } from "../../deployment-bundle/node-compat";
 import { FatalError } from "../../errors";
 import { logger } from "../../logger";
@@ -186,6 +187,10 @@ export async function deploy({
 		config?.compatibility_date ?? deploymentConfig.compatibility_date,
 		config?.compatibility_flags ?? deploymentConfig.compatibility_flags
 	);
+	const checkFetch = shouldCheckFetch(
+		config?.compatibility_date ?? deploymentConfig.compatibility_date,
+		config?.compatibility_flags ?? deploymentConfig.compatibility_flags
+	);
 
 	/**
 	 * Evaluate if this is an Advanced Mode or Pages Functions project. If Advanced Mode, we'll
@@ -221,6 +226,7 @@ export async function deploy({
 				local: false,
 				nodejsCompatMode,
 				defineNavigatorUserAgent,
+				checkFetch,
 			});
 
 			builtFunctions = readFileSync(
@@ -319,6 +325,7 @@ export async function deploy({
 			buildOutputDirectory: directory,
 			nodejsCompatMode,
 			defineNavigatorUserAgent,
+			checkFetch,
 			sourceMaps: sourceMaps,
 		});
 	} else if (_workerJS) {
@@ -337,6 +344,7 @@ export async function deploy({
 				onEnd: () => {},
 				nodejsCompatMode,
 				defineNavigatorUserAgent,
+				checkFetch,
 			});
 		} else {
 			await checkRawWorker(_workerPath, nodejsCompatMode, () => {});

packages/wrangler/src/api/startDevWorker/BundlerController.ts

@@ -3,7 +3,7 @@ import { readFileSync, realpathSync, writeFileSync } from "fs";
 import path from "path";
 import { watch } from "chokidar";
 import { noBundleWorker } from "../../deploy/deploy";
-import { bundleWorker } from "../../deployment-bundle/bundle";
+import { bundleWorker, shouldCheckFetch } from "../../deployment-bundle/bundle";
 import { getBundleType } from "../../deployment-bundle/bundle-type";
 import {
 	createModuleCollector,
@@ -117,7 +117,10 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {
 						minify: config.build.minify,
 						nodejsCompatMode: config.build.nodejsCompatMode,
 						define: config.build.define,
-						checkFetch: true,
+						checkFetch: shouldCheckFetch(
+							config.compatibilityDate,
+							config.compatibilityFlags
+						),
 						mockAnalyticsEngineDatasets:
 							bindings.analytics_engine_datasets ?? [],
 						alias: config.build.alias,
@@ -265,6 +268,10 @@ export class BundlerController extends Controller<BundlerControllerEventMap> {
 				onStart: () => {
 					this.emitBundleStartEvent(config);
 				},
+				checkFetch: shouldCheckFetch(
+					config.compatibilityDate,
+					config.compatibilityFlags
+				),
 				defineNavigatorUserAgent: isNavigatorDefined(
 					config.compatibilityDate,
 					config.compatibilityFlags

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

@@ -562,3 +562,26 @@ class BuildFailure extends Error {
 		super(message);
 	}
 }
+
+/**
+ * Whether to add middleware to check whether fetch requests use custom ports.
+ *
+ * This is controlled in the runtime by compatibility_flags:
+ *  - `ignore_custom_ports` - check fetch
+ *  - `allow_custom_ports` - do not check fetch
+ *
+ * `allow_custom_ports` became the default on 2024-09-02.
+ */
+export function shouldCheckFetch(
+	compatibilityDate: string = "2000-01-01", // Default to some arbitrary old date
+	compatibilityFlags: string[] = []
+): boolean {
+	// Yes, the logic can be less verbose than this but doing it this way makes it very clear.
+	if (compatibilityFlags.includes("ignore_custom_ports")) {
+		return true;
+	}
+	if (compatibilityFlags.includes("allow_custom_ports")) {
+		return false;
+	}
+	return compatibilityDate < "2024-09-02";
+}

packages/wrangler/src/dev/use-esbuild.ts

@@ -58,6 +58,7 @@ export function runBuild(
 		projectRoot,
 		onStart,
 		defineNavigatorUserAgent,
+		checkFetch,
 	}: {
 		entry: Entry;
 		destination: string | undefined;
@@ -84,6 +85,7 @@ export function runBuild(
 		projectRoot: string | undefined;
 		onStart: () => void;
 		defineNavigatorUserAgent: boolean;
+		checkFetch: boolean;
 	},
 	setBundle: (
 		cb: (previous: EsbuildBundle | undefined) => EsbuildBundle
@@ -158,7 +160,6 @@ export function runBuild(
 						workflowBindings: workflows,
 						alias,
 						define,
-						checkFetch: true,
 						mockAnalyticsEngineDatasets,
 						legacyAssets,
 						// disable the cache in dev
@@ -178,6 +179,7 @@ export function runBuild(
 
 						// sourcemap defaults to true in dev
 						sourcemap: undefined,
+						checkFetch,
 					})
 				: undefined;
 

packages/wrangler/src/pages/build.ts

@@ -9,6 +9,7 @@ import path, {
 } from "node:path";
 import { createUploadWorkerBundleContents } from "../api/pages/create-worker-bundle-contents";
 import { readConfig } from "../config";
+import { shouldCheckFetch } from "../deployment-bundle/bundle";
 import { writeAdditionalModules } from "../deployment-bundle/find-additional-modules";
 import { validateNodeCompatMode } from "../deployment-bundle/node-compat";
 import { FatalError } from "../errors";
@@ -152,6 +153,7 @@ export const Handler = async (args: PagesBuildArgs) => {
 			plugin,
 			nodejsCompatMode,
 			defineNavigatorUserAgent,
+			checkFetch,
 			external,
 		} = validatedArgs;
 
@@ -178,6 +180,7 @@ export const Handler = async (args: PagesBuildArgs) => {
 				routesOutputPath,
 				local: false,
 				defineNavigatorUserAgent,
+				checkFetch,
 				external,
 			});
 		} catch (e) {
@@ -219,6 +222,7 @@ export const Handler = async (args: PagesBuildArgs) => {
 			nodejsCompatMode,
 			workerScriptPath,
 			defineNavigatorUserAgent,
+			checkFetch,
 			external,
 		} = validatedArgs;
 
@@ -234,6 +238,7 @@ export const Handler = async (args: PagesBuildArgs) => {
 					buildOutputDirectory,
 					nodejsCompatMode,
 					defineNavigatorUserAgent,
+					checkFetch,
 					sourceMaps: config?.upload_source_maps ?? sourcemap,
 				});
 			} else {
@@ -252,6 +257,7 @@ export const Handler = async (args: PagesBuildArgs) => {
 					watch,
 					nodejsCompatMode,
 					defineNavigatorUserAgent,
+					checkFetch,
 					externalModules: external,
 				});
 			}
@@ -277,6 +283,7 @@ export const Handler = async (args: PagesBuildArgs) => {
 					routesOutputPath,
 					local: false,
 					defineNavigatorUserAgent,
+					checkFetch,
 					external,
 				});
 			} catch (e) {
@@ -320,6 +327,7 @@ type WorkerBundleArgs = Omit<PagesBuildArgs, "nodeCompat"> & {
 	buildOutputDirectory: string;
 	nodejsCompatMode: NodeJSCompatMode;
 	defineNavigatorUserAgent: boolean;
+	checkFetch: boolean;
 	workerScriptPath: string;
 	config: Config | undefined;
 	buildMetadata:
@@ -337,6 +345,7 @@ type PluginArgs = Omit<
 	outdir: string;
 	nodejsCompatMode: NodeJSCompatMode;
 	defineNavigatorUserAgent: boolean;
+	checkFetch: boolean;
 };
 async function maybeReadPagesConfig(
 	args: PagesBuildArgs
@@ -452,6 +461,11 @@ const validateArgs = async (args: PagesBuildArgs): Promise<ValidatedArgs> => {
 		args.compatibilityFlags
 	);
 
+	const checkFetch = shouldCheckFetch(
+		args.compatibilityDate,
+		args.compatibilityFlags
+	);
+
 	let workerScriptPath: string | undefined;
 
 	if (args.buildOutputDirectory) {
@@ -493,6 +507,7 @@ We looked for the Functions directory (${basename(
 		workerScriptPath,
 		nodejsCompatMode,
 		defineNavigatorUserAgent,
+		checkFetch,
 		config,
 		buildMetadata:
 			config && args.projectDirectory && config.pages_build_output_dir

packages/wrangler/src/pages/buildFunctions.ts

@@ -39,6 +39,7 @@ export async function buildFunctions({
 		`./functionsRoutes-${Math.random()}.mjs`
 	),
 	defineNavigatorUserAgent,
+	checkFetch,
 	external,
 }: Partial<
 	Pick<
@@ -64,6 +65,7 @@ export async function buildFunctions({
 	// temporary directory each time
 	routesModule?: string;
 	defineNavigatorUserAgent: boolean;
+	checkFetch: boolean;
 }) {
 	RUNNING_BUILDERS.forEach(
 		(runningBuilder) => runningBuilder.stop && runningBuilder.stop()
@@ -121,6 +123,7 @@ export async function buildFunctions({
 			functionsDirectory: absoluteFunctionsDirectory,
 			local,
 			defineNavigatorUserAgent,
+			checkFetch,
 			external,
 		});
 	} else {
@@ -138,6 +141,7 @@ export async function buildFunctions({
 			buildOutputDirectory,
 			nodejsCompatMode,
 			defineNavigatorUserAgent,
+			checkFetch,
 			external,
 		});
 	}

packages/wrangler/src/pages/dev.ts

@@ -6,6 +6,7 @@ import * as esbuild from "esbuild";
 import { unstable_dev } from "../api";
 import { readConfig } from "../config";
 import { isBuildFailure } from "../deployment-bundle/build-failures";
+import { shouldCheckFetch } from "../deployment-bundle/bundle";
 import { esbuildAliasExternalPlugin } from "../deployment-bundle/esbuild-plugins/alias-external";
 import { validateNodeCompatMode } from "../deployment-bundle/node-compat";
 import { FatalError } from "../errors";
@@ -384,6 +385,9 @@ export const Handler = async (args: PagesDevArguments) => {
 		compatibilityDate,
 		compatibilityFlags
 	);
+
+	const checkFetch = shouldCheckFetch(compatibilityDate, compatibilityFlags);
+
 	let modules: CfModule[] = [];
 
 	if (usingWorkerDirectory) {
@@ -394,6 +398,7 @@ export const Handler = async (args: PagesDevArguments) => {
 				buildOutputDirectory: directory ?? ".",
 				nodejsCompatMode,
 				defineNavigatorUserAgent,
+				checkFetch,
 				sourceMaps: config?.upload_source_maps ?? false,
 			});
 			modules = bundleResult.modules;
@@ -462,6 +467,7 @@ export const Handler = async (args: PagesDevArguments) => {
 					watch: false,
 					onEnd: () => scriptReadyResolve(),
 					defineNavigatorUserAgent,
+					checkFetch,
 				});
 
 				/*
@@ -632,6 +638,7 @@ export const Handler = async (args: PagesDevArguments) => {
 				local: true,
 				routesModule,
 				defineNavigatorUserAgent,
+				checkFetch,
 			});
 
 			/*

packages/wrangler/src/pages/functions/buildPlugin.ts

@@ -24,6 +24,7 @@ export function buildPluginFromFunctions({
 	functionsDirectory,
 	local,
 	defineNavigatorUserAgent,
+	checkFetch,
 	external,
 }: Options) {
 	const entry: Entry = {
@@ -107,7 +108,7 @@ export function buildPluginFromFunctions({
 			},
 		],
 		serveLegacyAssetsFromWorker: false,
-		checkFetch: local,
+		checkFetch: local && checkFetch,
 		// TODO: mock AE datasets in Pages functions for dev
 		mockAnalyticsEngineDatasets: [],
 		targetConsumer: local ? "dev" : "deploy",