From e2e6912bcb7a1f6b7f8081b889a4e08be8a740a1 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Mon, 18 Nov 2024 12:24:09 +0000 Subject: [PATCH] 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 https://github.com/cloudflare/workerd/issues/2955 * fix: typo --- .changeset/shaggy-monkeys-visit.md | 9 ++++++ .../src/__tests__/check-fetch.test.ts | 28 +++++++++++++++++++ packages/wrangler/src/api/pages/deploy.ts | 8 ++++++ .../api/startDevWorker/BundlerController.ts | 11 ++++++-- .../wrangler/src/deployment-bundle/bundle.ts | 23 +++++++++++++++ packages/wrangler/src/dev/use-esbuild.ts | 4 ++- packages/wrangler/src/pages/build.ts | 15 ++++++++++ packages/wrangler/src/pages/buildFunctions.ts | 4 +++ packages/wrangler/src/pages/dev.ts | 7 +++++ .../src/pages/functions/buildPlugin.ts | 3 +- .../src/pages/functions/buildWorker.ts | 11 ++++++-- 11 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 .changeset/shaggy-monkeys-visit.md create mode 100644 packages/wrangler/src/__tests__/check-fetch.test.ts 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/api/pages/deploy.ts b/packages/wrangler/src/api/pages/deploy.ts index e8b3333f294f..fcdb6d0ef987 100644 --- a/packages/wrangler/src/api/pages/deploy.ts +++ b/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, () => {}); diff --git a/packages/wrangler/src/api/startDevWorker/BundlerController.ts b/packages/wrangler/src/api/startDevWorker/BundlerController.ts index bf22f524a521..bf633397473f 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, @@ -117,7 +117,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, @@ -265,6 +268,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 8fafb78f0d1c..dd7d16715568 100644 --- a/packages/wrangler/src/deployment-bundle/bundle.ts +++ b/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"; +} diff --git a/packages/wrangler/src/dev/use-esbuild.ts b/packages/wrangler/src/dev/use-esbuild.ts index 42d422aab2d1..c93018ee3062 100644 --- a/packages/wrangler/src/dev/use-esbuild.ts +++ b/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; 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 6fd58827e550..881b16d31d8d 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"; @@ -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, }); /* diff --git a/packages/wrangler/src/pages/functions/buildPlugin.ts b/packages/wrangler/src/pages/functions/buildPlugin.ts index 11f5234744c6..04bef665e0d6 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 = { @@ -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", diff --git a/packages/wrangler/src/pages/functions/buildWorker.ts b/packages/wrangler/src/pages/functions/buildWorker.ts index 44db17ec84f3..0320ba09ead9 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 = { @@ -85,7 +87,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(), @@ -117,6 +119,7 @@ export type RawOptions = { local: boolean; additionalModules?: CfModule[]; defineNavigatorUserAgent: boolean; + checkFetch: boolean; external?: string[]; }; @@ -143,6 +146,7 @@ export function buildRawWorker({ local, additionalModules = [], defineNavigatorUserAgent, + checkFetch, external, }: RawOptions) { const entry: Entry = { @@ -197,7 +201,7 @@ export function buildRawWorker({ ], isOutfile: !outdir, serveLegacyAssetsFromWorker: false, - checkFetch: local, + checkFetch: local && checkFetch, targetConsumer: local ? "dev" : "deploy", local, projectRoot: getPagesProjectRoot(), @@ -220,6 +224,7 @@ export async function produceWorkerBundleForWorkerJSDirectory({ buildOutputDirectory, nodejsCompatMode, defineNavigatorUserAgent, + checkFetch, sourceMaps, }: { workerJSDirectory: string; @@ -227,6 +232,7 @@ export async function produceWorkerBundleForWorkerJSDirectory({ buildOutputDirectory: string; nodejsCompatMode: NodeJSCompatMode; defineNavigatorUserAgent: boolean; + checkFetch: boolean; sourceMaps: boolean; }): Promise { const entrypoint = resolve(join(workerJSDirectory, "index.js")); @@ -278,6 +284,7 @@ export async function produceWorkerBundleForWorkerJSDirectory({ nodejsCompatMode, additionalModules, defineNavigatorUserAgent, + checkFetch, }); return { modules: bundleResult.modules,