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

.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/__tests__/validate-dev-props.test.ts

@@ -1,3 +1,4 @@
+import { compatibilityDate } from "workerd";
 import { validateDevProps } from "../dev/validate-dev-props";
 import type { DevProps } from "../dev/dev";
 

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

@@ -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,
@@ -116,7 +116,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,
@@ -249,6 +252,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

@@ -567,3 +567,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 is 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/dev.tsx

@@ -14,6 +14,7 @@ import {
 	createDeferred,
 	fakeResolvedInput,
 } from "../api/startDevWorker/utils";
+import { shouldCheckFetch } from "../deployment-bundle/bundle";
 import { runCustomBuild } from "../deployment-bundle/run-custom-build";
 import {
 	getBoundRegisteredWorkers,
@@ -555,6 +556,10 @@ function DevSession(props: DevSessionProps) {
 			props.compatibilityDate,
 			props.compatibilityFlags
 		),
+		checkFetch: shouldCheckFetch(
+			props.compatibilityDate,
+			props.compatibilityFlags
+		),
 	});
 
 	// TODO(queues) support remote wrangler dev

packages/wrangler/src/dev/start-server.ts

@@ -8,7 +8,7 @@ import {
 	convertCfWorkerInitBindingstoBindings,
 	fakeResolvedInput,
 } from "../api/startDevWorker/utils";
-import { bundleWorker } from "../deployment-bundle/bundle";
+import { bundleWorker, shouldCheckFetch } from "../deployment-bundle/bundle";
 import { getBundleType } from "../deployment-bundle/bundle-type";
 import { dedupeModulesByName } from "../deployment-bundle/dedupe-modules";
 import { findAdditionalModules as doFindAdditionalModules } from "../deployment-bundle/find-additional-modules";
@@ -239,6 +239,10 @@ export async function startDevServer(
 		tsconfig: props.tsconfig,
 		minify: props.minify,
 		nodejsCompatMode: props.nodejsCompatMode,
+		checkFetch: shouldCheckFetch(
+			props.compatibilityDate,
+			props.compatibilityFlags
+		),
 		define: props.define,
 		noBundle: props.noBundle,
 		findAdditionalModules: props.findAdditionalModules,
@@ -416,6 +420,7 @@ async function runEsbuild({
 	tsconfig,
 	minify,
 	nodejsCompatMode,
+	checkFetch,
 	define,
 	noBundle,
 	findAdditionalModules,
@@ -441,6 +446,7 @@ async function runEsbuild({
 	tsconfig: string | undefined;
 	minify: boolean | undefined;
 	nodejsCompatMode: NodeJSCompatMode | undefined;
+	checkFetch: boolean;
 	noBundle: boolean;
 	findAdditionalModules: boolean | undefined;
 	testScheduled?: boolean;
@@ -484,7 +490,7 @@ async function runEsbuild({
 					minify,
 					nodejsCompatMode,
 					define,
-					checkFetch: true,
+					checkFetch,
 					mockAnalyticsEngineDatasets,
 					alias,
 					legacyAssets,

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

@@ -60,6 +60,7 @@ export type EsbuildBundleProps = {
 	onStart: () => void;
 	onComplete: (bundle: EsbuildBundle) => void;
 	defineNavigatorUserAgent: boolean;
+	checkFetch: boolean;
 };
 
 export function runBuild(
@@ -89,6 +90,7 @@ export function runBuild(
 		projectRoot,
 		onStart,
 		defineNavigatorUserAgent,
+		checkFetch,
 	}: {
 		entry: Entry;
 		destination: string | undefined;
@@ -115,6 +117,7 @@ export function runBuild(
 		projectRoot: string | undefined;
 		onStart: () => void;
 		defineNavigatorUserAgent: boolean;
+		checkFetch: boolean;
 	},
 	setBundle: (
 		cb: (previous: EsbuildBundle | undefined) => EsbuildBundle
@@ -189,7 +192,6 @@ export function runBuild(
 						workflowBindings: workflows,
 						alias,
 						define,
-						checkFetch: true,
 						mockAnalyticsEngineDatasets,
 						legacyAssets,
 						// disable the cache in dev
@@ -200,6 +202,7 @@ export function runBuild(
 						local,
 						projectRoot,
 						defineNavigatorUserAgent,
+						checkFetch,
 					})
 				: undefined;
 
@@ -282,6 +285,7 @@ export function useEsbuild({
 	onStart,
 	onComplete,
 	defineNavigatorUserAgent,
+	checkFetch,
 }: EsbuildBundleProps): EsbuildBundle | undefined {
 	const [bundle, setBundle] = useState<EsbuildBundle>();
 	const { exit } = useApp();
@@ -313,6 +317,7 @@ export function useEsbuild({
 				projectRoot,
 				onStart,
 				defineNavigatorUserAgent,
+				checkFetch,
 			},
 			setBundle,
 			(err) => exit(err)
@@ -348,6 +353,7 @@ export function useEsbuild({
 		projectRoot,
 		onStart,
 		defineNavigatorUserAgent,
+		checkFetch,
 	]);
 
 	useEffect(() => {

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