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
cloudflare · Aug 28, 2024 · 510898e · 510898e
1 parent 3e40999
commit 510898e
Show file tree

Hide file tree

Showing 8 changed files with 100 additions and 88 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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<ValidatedArgs> => {
 	}
 
 	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
@@ -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
@@ -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
@@ -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 =