From 510898e452cc4c68c6ac2369b62d27914e103e34 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Wed, 28 Aug 2024 15:06:12 +0200 Subject: [PATCH] fixup PR: Rename validateNodeCompat to getNodeCompatMode which is what the function does Convert the parameters to optional options for better readability. Added a `validateConfig` to be able to skip validation (used once in the code base). The former getNodeCompatMode is now parseNodeCompatibilityFlags and is only responsible to return the compat flags and no more exported. Improve the documentation --- packages/wrangler/src/api/pages/deploy.tsx | 10 +- packages/wrangler/src/deploy/deploy.ts | 11 +- .../src/deployment-bundle/node-compat.ts | 102 +++++++++--------- packages/wrangler/src/dev.tsx | 26 +++-- packages/wrangler/src/pages/build.ts | 11 +- packages/wrangler/src/pages/dev.ts | 10 +- .../wrangler/src/type-generation/index.ts | 8 +- packages/wrangler/src/versions/upload.ts | 10 +- 8 files changed, 100 insertions(+), 88 deletions(-) diff --git a/packages/wrangler/src/api/pages/deploy.tsx b/packages/wrangler/src/api/pages/deploy.tsx index b73faff9105e..b1e270881b95 100644 --- a/packages/wrangler/src/api/pages/deploy.tsx +++ b/packages/wrangler/src/api/pages/deploy.tsx @@ -6,7 +6,7 @@ import { cwd } from "node:process"; import { File, FormData } from "undici"; import { fetchResult } from "../../cfetch"; import { readConfig } from "../../config"; -import { validateNodeCompat } from "../../deployment-bundle/node-compat"; +import { getNodeCompatMode } from "../../deployment-bundle/node-compat"; import { FatalError } from "../../errors"; import { logger } from "../../logger"; import { isNavigatorDefined } from "../../navigator-user-agent"; @@ -174,10 +174,12 @@ export async function deploy({ } } - const nodejsCompatMode = validateNodeCompat( + const nodejsCompatMode = getNodeCompatMode( config?.compatibility_flags ?? deploymentConfig.compatibility_flags ?? [], - /* node_compat */ false, - config?.no_bundle + { + nodeCompat: false, + noBundle: config?.no_bundle, + } ); const defineNavigatorUserAgent = isNavigatorDefined( config?.compatibility_date ?? deploymentConfig.compatibility_date, diff --git a/packages/wrangler/src/deploy/deploy.ts b/packages/wrangler/src/deploy/deploy.ts index b5e80afdda29..b125950eb6fe 100644 --- a/packages/wrangler/src/deploy/deploy.ts +++ b/packages/wrangler/src/deploy/deploy.ts @@ -21,7 +21,7 @@ import { createModuleCollector, getWrangler1xLegacyModuleReferences, } from "../deployment-bundle/module-collection"; -import { validateNodeCompat } from "../deployment-bundle/node-compat"; +import { getNodeCompatMode } from "../deployment-bundle/node-compat"; import { loadSourceMaps } from "../deployment-bundle/source-maps"; import { addHyphens } from "../deployments"; import { confirm } from "../dialogs"; @@ -399,11 +399,10 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m const compatibilityFlags = props.compatibilityFlags ?? config.compatibility_flags; - const nodejsCompatMode = validateNodeCompat( - compatibilityFlags, - props.nodeCompat ?? config.node_compat, - props.noBundle ?? config.no_bundle - ); + const nodejsCompatMode = getNodeCompatMode(compatibilityFlags, { + nodeCompat: props.nodeCompat ?? config.node_compat, + noBundle: props.noBundle ?? config.no_bundle, + }); // Warn if user tries minify with no-bundle if (props.noBundle && minify) { diff --git a/packages/wrangler/src/deployment-bundle/node-compat.ts b/packages/wrangler/src/deployment-bundle/node-compat.ts index d2b4c03147bd..d0da897bd38a 100644 --- a/packages/wrangler/src/deployment-bundle/node-compat.ts +++ b/packages/wrangler/src/deployment-bundle/node-compat.ts @@ -11,49 +11,72 @@ import { logger } from "../logger"; export type NodeJSCompatMode = "legacy" | "v1" | "v2" | null; /** - * Validate and compute the Node.js compatibility mode we are running. + * Computes the Node.js compatibility mode we are running. * - * Returns one of: + * NOTE: + * Currently v2 mode is configured via `nodejs_compat_v2` compat flag. + * At a future compatibility date, the use of `nodejs_compat` flag will imply `nodejs_compat_v2`. + * + * see `EnvironmentInheritable` for `nodeCompat` and `noBundle`. + * + * @param compatibilityFlags The compatibility flags + * @param validateConfig Whether to validate the config (logs and throws) + * @param nodeCompat Whether to add polyfills for node builtin modules and globals + * @param noBundle Whether to skip internal build steps and directly deploy script + * @returns one of: * - "legacy": build-time polyfills, from `node_compat` flag * - "v1": nodejs_compat compatibility flag * - "v2": nodejs_compat_v2 compatibility flag * - null: no Node.js compatibility - * - * Currently v2 mode is configured via `nodejs_compat_v2` compat flag. - * At a future compatibility date, the use of `nodejs_compat` flag will imply `nodejs_compat_v2`. - * - * We assert that only one of these modes can be specified at a time. - * We assert that you must prefix v2 mode with `experimental`. - * We warn if using legacy or v2 mode. */ -export function validateNodeCompat( +export function getNodeCompatMode( compatibilityFlags: string[], - nodeCompat: boolean | undefined, - noBundle: boolean | undefined + { + validateConfig = true, + nodeCompat = undefined, + noBundle = undefined, + }: { + validateConfig?: boolean; + nodeCompat?: boolean; + noBundle?: boolean; + } ): NodeJSCompatMode { const { - mode, - nodejsCompat, - nodejsCompatV2, - experimentalNodejsCompatV2, - legacy, - } = getNodeCompatMode(compatibilityFlags, nodeCompat); + hasNodejsCompatFlag, + hasNodejsCompatV2Flag, + hasExperimentalNodejsCompatV2Flag, + } = parseNodeCompatibilityFlags(compatibilityFlags); - if (experimentalNodejsCompatV2) { + const legacy = nodeCompat === true; + let mode: NodeJSCompatMode = null; + if (hasNodejsCompatV2Flag) { + mode = "v2"; + } else if (hasNodejsCompatFlag) { + mode = "v1"; + } else if (legacy) { + mode = "legacy"; + } + + if (validateConfig !== true) { + // Skip the validation. + return mode; + } + + if (hasExperimentalNodejsCompatV2Flag) { throw new UserError( "The `experimental:` prefix on `nodejs_compat_v2` is no longer valid. Please remove it and try again." ); } - if (nodejsCompat && nodejsCompatV2) { + if (hasNodejsCompatFlag && hasNodejsCompatV2Flag) { throw new UserError( "The `nodejs_compat` and `nodejs_compat_v2` compatibility flags cannot be used in together. Please select just one." ); } - if (legacy && (nodejsCompat || nodejsCompatV2)) { + if (legacy && (hasNodejsCompatFlag || hasNodejsCompatV2Flag)) { throw new UserError( - `The ${nodejsCompat ? "`nodejs_compat`" : "`nodejs_compat_v2`"} compatibility flag cannot be used in conjunction with the legacy \`--node-compat\` flag. If you want to use the Workers ${nodejsCompat ? "`nodejs_compat`" : "`nodejs_compat_v2`"} compatibility flag, please remove the \`--node-compat\` argument from your CLI command or \`node_compat = true\` from your config file.` + `The ${hasNodejsCompatFlag ? "`nodejs_compat`" : "`nodejs_compat_v2`"} compatibility flag cannot be used in conjunction with the legacy \`--node-compat\` flag. If you want to use the Workers ${hasNodejsCompatFlag ? "`nodejs_compat`" : "`nodejs_compat_v2`"} compatibility flag, please remove the \`--node-compat\` argument from your CLI command or \`node_compat = true\` from your config file.` ); } @@ -63,7 +86,7 @@ export function validateNodeCompat( ); } - if (noBundle && nodejsCompatV2) { + if (noBundle && hasNodejsCompatV2Flag) { logger.warn( "`nodejs_compat_v2` compatibility flag and `--no-bundle` can't be used together. If you want to polyfill Node.js built-ins and disable Wrangler's bundling, please polyfill as part of your own bundling process." ); @@ -78,33 +101,12 @@ export function validateNodeCompat( return mode; } -export function getNodeCompatMode( - compatibilityFlags: string[], - nodeCompat: boolean | undefined -) { - const legacy = nodeCompat === true; - const nodejsCompat = compatibilityFlags.includes("nodejs_compat"); - const nodejsCompatV2 = compatibilityFlags.includes("nodejs_compat_v2"); - const experimentalNodejsCompatV2 = compatibilityFlags.includes( - "experimental:nodejs_compat_v2" - ); - - let mode: NodeJSCompatMode; - if (nodejsCompatV2) { - mode = "v2"; - } else if (nodejsCompat) { - mode = "v1"; - } else if (legacy) { - mode = "legacy"; - } else { - mode = null; - } - +function parseNodeCompatibilityFlags(compatibilityFlags: string[]) { return { - legacy, - mode, - nodejsCompat, - nodejsCompatV2, - experimentalNodejsCompatV2, + hasNodejsCompatFlag: compatibilityFlags.includes("nodejs_compat"), + hasNodejsCompatV2Flag: compatibilityFlags.includes("nodejs_compat_v2"), + hasExperimentalNodejsCompatV2Flag: compatibilityFlags.includes( + "experimental:nodejs_compat_v2" + ), }; } diff --git a/packages/wrangler/src/dev.tsx b/packages/wrangler/src/dev.tsx index 635c0becab89..10395a3017ce 100644 --- a/packages/wrangler/src/dev.tsx +++ b/packages/wrangler/src/dev.tsx @@ -12,7 +12,7 @@ import { } from "./api/startDevWorker/utils"; import { findWranglerToml, printBindings, readConfig } from "./config"; import { getEntry } from "./deployment-bundle/entry"; -import { validateNodeCompat } from "./deployment-bundle/node-compat"; +import { getNodeCompatMode } from "./deployment-bundle/node-compat"; import { getBoundRegisteredWorkers } from "./dev-registry"; import Dev, { devRegistry } from "./dev/dev"; import { getVarsForDev } from "./dev/dev-vars"; @@ -680,10 +680,12 @@ export async function startDev(args: StartDevOptions) { moduleRoot: args.moduleRoot, moduleRules: args.rules, nodejsCompatMode: (parsedConfig: Config) => - validateNodeCompat( + getNodeCompatMode( args.compatibilityFlags ?? parsedConfig.compatibility_flags ?? [], - args.nodeCompat ?? parsedConfig.node_compat, - args.noBundle ?? parsedConfig.no_bundle + { + nodeCompat: args.nodeCompat ?? parsedConfig.node_compat, + noBundle: args.noBundle ?? parsedConfig.no_bundle, + } ), }, bindings: { @@ -829,10 +831,12 @@ export async function startDev(args: StartDevOptions) { additionalModules, } = await validateDevServerSettings(args, config); - const nodejsCompatMode = validateNodeCompat( + const nodejsCompatMode = getNodeCompatMode( args.compatibilityFlags ?? config.compatibility_flags ?? [], - args.nodeCompat ?? config.node_compat, - args.noBundle ?? config.no_bundle + { + nodeCompat: args.nodeCompat ?? config.node_compat, + noBundle: args.noBundle ?? config.no_bundle, + } ); void metrics.sendMetricsEvent( @@ -968,10 +972,12 @@ export async function startApiDev(args: StartDevOptions) { additionalModules, } = await validateDevServerSettings(args, config); - const nodejsCompatMode = validateNodeCompat( + const nodejsCompatMode = getNodeCompatMode( args.compatibilityFlags ?? config.compatibility_flags, - args.nodeCompat ?? config.node_compat, - args.noBundle ?? config.no_bundle + { + nodeCompat: args.nodeCompat ?? config.node_compat, + noBundle: args.noBundle ?? config.no_bundle, + } ); await metrics.sendMetricsEvent( diff --git a/packages/wrangler/src/pages/build.ts b/packages/wrangler/src/pages/build.ts index 1e4f4ebf76ad..006d26d83130 100644 --- a/packages/wrangler/src/pages/build.ts +++ b/packages/wrangler/src/pages/build.ts @@ -10,7 +10,7 @@ import path, { import { createUploadWorkerBundleContents } from "../api/pages/create-worker-bundle-contents"; import { readConfig } from "../config"; import { writeAdditionalModules } from "../deployment-bundle/find-additional-modules"; -import { validateNodeCompat } from "../deployment-bundle/node-compat"; +import { getNodeCompatMode } from "../deployment-bundle/node-compat"; import { FatalError } from "../errors"; import { logger } from "../logger"; import * as metrics from "../metrics"; @@ -438,11 +438,10 @@ const validateArgs = async (args: PagesBuildArgs): Promise => { } const { nodeCompat: node_compat, ...argsExceptNodeCompat } = args; - const nodejsCompatMode = validateNodeCompat( - args.compatibilityFlags ?? [], - node_compat, - config?.no_bundle - ); + const nodejsCompatMode = getNodeCompatMode(args.compatibilityFlags ?? [], { + nodeCompat: node_compat, + noBundle: config?.no_bundle, + }); const defineNavigatorUserAgent = isNavigatorDefined( args.compatibilityDate, diff --git a/packages/wrangler/src/pages/dev.ts b/packages/wrangler/src/pages/dev.ts index 077804758d32..6d30ccce5050 100644 --- a/packages/wrangler/src/pages/dev.ts +++ b/packages/wrangler/src/pages/dev.ts @@ -7,7 +7,7 @@ import { unstable_dev } from "../api"; import { readConfig } from "../config"; import { isBuildFailure } from "../deployment-bundle/build-failures"; import { esbuildAliasExternalPlugin } from "../deployment-bundle/esbuild-plugins/alias-external"; -import { validateNodeCompat } from "../deployment-bundle/node-compat"; +import { getNodeCompatMode } from "../deployment-bundle/node-compat"; import { FatalError } from "../errors"; import { logger } from "../logger"; import * as metrics from "../metrics"; @@ -360,10 +360,12 @@ export const Handler = async (args: PagesDevArguments) => { let scriptPath = ""; - const nodejsCompatMode = validateNodeCompat( + const nodejsCompatMode = getNodeCompatMode( args.compatibilityFlags ?? config.compatibility_flags ?? [], - args.nodeCompat, - args.noBundle ?? config.no_bundle + { + nodeCompat: args.nodeCompat, + noBundle: args.noBundle ?? config.no_bundle, + } ); const defineNavigatorUserAgent = isNavigatorDefined( diff --git a/packages/wrangler/src/type-generation/index.ts b/packages/wrangler/src/type-generation/index.ts index d865abef7be0..4e97dabe9b73 100644 --- a/packages/wrangler/src/type-generation/index.ts +++ b/packages/wrangler/src/type-generation/index.ts @@ -92,10 +92,10 @@ export async function typesHandler( const tsconfigPath = config.tsconfig ?? join(dirname(configPath), "tsconfig.json"); const tsconfigTypes = readTsconfigTypes(tsconfigPath); - const { mode } = getNodeCompatMode( - config.compatibility_flags, - config.node_compat - ); + const mode = getNodeCompatMode(config.compatibility_flags, { + validateConfig: false, + nodeCompat: config.node_compat, + }); logRuntimeTypesMessage(outFile, tsconfigTypes, mode !== null); } diff --git a/packages/wrangler/src/versions/upload.ts b/packages/wrangler/src/versions/upload.ts index a09d3ea1d070..e5b1ffd10bfd 100644 --- a/packages/wrangler/src/versions/upload.ts +++ b/packages/wrangler/src/versions/upload.ts @@ -19,7 +19,7 @@ import { createModuleCollector, getWrangler1xLegacyModuleReferences, } from "../deployment-bundle/module-collection"; -import { validateNodeCompat } from "../deployment-bundle/node-compat"; +import { getNodeCompatMode } from "../deployment-bundle/node-compat"; import { loadSourceMaps } from "../deployment-bundle/source-maps"; import { confirm } from "../dialogs"; import { getMigrationsToUpload } from "../durable"; @@ -184,10 +184,12 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m const minify = props.minify ?? config.minify; - const nodejsCompatMode = validateNodeCompat( + const nodejsCompatMode = getNodeCompatMode( props.compatibilityFlags ?? config.compatibility_flags, - props.nodeCompat ?? config.node_compat, - props.noBundle ?? config.no_bundle + { + nodeCompat: props.nodeCompat ?? config.node_compat, + noBundle: props.noBundle ?? config.no_bundle, + } ); const compatibilityFlags =