diff --git a/.changeset/shaggy-monkeys-visit.md b/.changeset/shaggy-monkeys-visit.md new file mode 100644 index 000000000000..c390c17c1d8e --- /dev/null +++ b/.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 diff --git a/packages/wrangler/src/__tests__/check-fetch.test.ts b/packages/wrangler/src/__tests__/check-fetch.test.ts new file mode 100644 index 000000000000..8d17009b35b0 --- /dev/null +++ b/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); + }); +}); diff --git a/packages/wrangler/src/__tests__/validate-dev-props.test.ts b/packages/wrangler/src/__tests__/validate-dev-props.test.ts index 434cbf04653d..55aa5da623c7 100644 --- a/packages/wrangler/src/__tests__/validate-dev-props.test.ts +++ b/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"; diff --git a/packages/wrangler/src/api/pages/deploy.tsx b/packages/wrangler/src/api/pages/deploy.tsx index e8b3333f294f..fcdb6d0ef987 100644 --- a/packages/wrangler/src/api/pages/deploy.tsx +++ b/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, () => {}); diff --git a/packages/wrangler/src/api/startDevWorker/BundlerController.ts b/packages/wrangler/src/api/startDevWorker/BundlerController.ts index 4e7f8ee5524e..a34d26751509 100644 --- a/packages/wrangler/src/api/startDevWorker/BundlerController.ts +++ b/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 { 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 { onStart: () => { this.emitBundleStartEvent(config); }, + checkFetch: shouldCheckFetch( + config.compatibilityDate, + config.compatibilityFlags + ), defineNavigatorUserAgent: isNavigatorDefined( config.compatibilityDate, config.compatibilityFlags diff --git a/packages/wrangler/src/deployment-bundle/bundle.ts b/packages/wrangler/src/deployment-bundle/bundle.ts index 3727715c8f5f..7ae479271c1b 100644 --- a/packages/wrangler/src/deployment-bundle/bundle.ts +++ b/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"; +} diff --git a/packages/wrangler/src/dev/dev.tsx b/packages/wrangler/src/dev/dev.tsx index e8210b851eba..af30d859eb65 100644 --- a/packages/wrangler/src/dev/dev.tsx +++ b/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 diff --git a/packages/wrangler/src/dev/start-server.ts b/packages/wrangler/src/dev/start-server.ts index 913800d57717..e7a32fab6a0d 100644 --- a/packages/wrangler/src/dev/start-server.ts +++ b/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, diff --git a/packages/wrangler/src/dev/use-esbuild.ts b/packages/wrangler/src/dev/use-esbuild.ts index 6aa2545f3437..30aaaa37d5f0 100644 --- a/packages/wrangler/src/dev/use-esbuild.ts +++ b/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(); 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(() => { diff --git a/packages/wrangler/src/pages/build.ts b/packages/wrangler/src/pages/build.ts index 35f7c303b970..958aa9102f88 100644 --- a/packages/wrangler/src/pages/build.ts +++ b/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 & { 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 => { 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 diff --git a/packages/wrangler/src/pages/buildFunctions.ts b/packages/wrangler/src/pages/buildFunctions.ts index 4f1a2263b695..8344bfd54e07 100644 --- a/packages/wrangler/src/pages/buildFunctions.ts +++ b/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, }); } diff --git a/packages/wrangler/src/pages/dev.ts b/packages/wrangler/src/pages/dev.ts index 630e4bc71a41..0becba121216 100644 --- a/packages/wrangler/src/pages/dev.ts +++ b/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"; @@ -373,6 +374,9 @@ export const Handler = async (args: PagesDevArguments) => { compatibilityDate, compatibilityFlags ); + + const checkFetch = shouldCheckFetch(compatibilityDate, compatibilityFlags); + let modules: CfModule[] = []; if (usingWorkerDirectory) { @@ -383,6 +387,7 @@ export const Handler = async (args: PagesDevArguments) => { buildOutputDirectory: directory ?? ".", nodejsCompatMode, defineNavigatorUserAgent, + checkFetch, sourceMaps: config?.upload_source_maps ?? false, }); modules = bundleResult.modules; @@ -451,6 +456,7 @@ export const Handler = async (args: PagesDevArguments) => { watch: false, onEnd: () => scriptReadyResolve(), defineNavigatorUserAgent, + checkFetch, }); /* @@ -621,6 +627,7 @@ export const Handler = async (args: PagesDevArguments) => { local: true, routesModule, defineNavigatorUserAgent, + checkFetch, }); /* diff --git a/packages/wrangler/src/pages/functions/buildPlugin.ts b/packages/wrangler/src/pages/functions/buildPlugin.ts index 7f6c1b022b2e..9d91a741a0b9 100644 --- a/packages/wrangler/src/pages/functions/buildPlugin.ts +++ b/packages/wrangler/src/pages/functions/buildPlugin.ts @@ -24,6 +24,7 @@ export function buildPluginFromFunctions({ functionsDirectory, local, defineNavigatorUserAgent, + checkFetch, external, }: Options) { const entry: Entry = { @@ -106,7 +107,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", diff --git a/packages/wrangler/src/pages/functions/buildWorker.ts b/packages/wrangler/src/pages/functions/buildWorker.ts index d6f670f60f25..762b06c9e0b2 100644 --- a/packages/wrangler/src/pages/functions/buildWorker.ts +++ b/packages/wrangler/src/pages/functions/buildWorker.ts @@ -32,6 +32,7 @@ export type Options = { functionsDirectory: string; local: boolean; defineNavigatorUserAgent: boolean; + checkFetch: boolean; external?: string[]; }; @@ -49,6 +50,7 @@ export function buildWorkerFromFunctions({ functionsDirectory, local, defineNavigatorUserAgent, + checkFetch, external, }: Options) { const entry: Entry = { @@ -84,7 +86,7 @@ export function buildWorkerFromFunctions({ plugins: [buildNotifierPlugin(onEnd), assetsPlugin(buildOutputDirectory)], isOutfile: !outdir, serveLegacyAssetsFromWorker: false, - checkFetch: local, + checkFetch: local && checkFetch, targetConsumer: local ? "dev" : "deploy", local, projectRoot: getPagesProjectRoot(), @@ -109,6 +111,7 @@ export type RawOptions = { local: boolean; additionalModules?: CfModule[]; defineNavigatorUserAgent: boolean; + checkFetch: boolean; external?: string[]; }; @@ -135,6 +138,7 @@ export function buildRawWorker({ local, additionalModules = [], defineNavigatorUserAgent, + checkFetch, external, }: RawOptions) { const entry: Entry = { @@ -188,7 +192,7 @@ export function buildRawWorker({ ], isOutfile: !outdir, serveLegacyAssetsFromWorker: false, - checkFetch: local, + checkFetch: local && checkFetch, targetConsumer: local ? "dev" : "deploy", local, projectRoot: getPagesProjectRoot(), @@ -202,6 +206,7 @@ export async function produceWorkerBundleForWorkerJSDirectory({ buildOutputDirectory, nodejsCompatMode, defineNavigatorUserAgent, + checkFetch, sourceMaps, }: { workerJSDirectory: string; @@ -209,6 +214,7 @@ export async function produceWorkerBundleForWorkerJSDirectory({ buildOutputDirectory: string; nodejsCompatMode: NodeJSCompatMode; defineNavigatorUserAgent: boolean; + checkFetch: boolean; sourceMaps: boolean; }): Promise { const entrypoint = resolve(join(workerJSDirectory, "index.js")); @@ -259,6 +265,7 @@ export async function produceWorkerBundleForWorkerJSDirectory({ nodejsCompatMode, additionalModules, defineNavigatorUserAgent, + checkFetch, }); return { modules: bundleResult.modules,