fix: remove experimental: prefix requirement for nodejs_compat_v2 (#6593

)

* fix: remove `experimental:` prefix requirement for nodejs_compat_v2

See https://jira.cfdata.org/browse/DEVDASH-218

* PR review updates 2

* PR review udpdates

* add error message for invalid use of `experimental` prefix

* reworking parameters after feedback

* 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

---------

Co-authored-by: Peter Bacon Darwin <pbacondarwin@cloudflare.com>

.changeset/green-lies-crash.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: remove `experimental:` prefix requirement for nodejs_compat_v2
+
+See https://jira.cfdata.org/browse/DEVDASH-218

fixtures/nodejs-hybrid-app/wrangler.toml

@@ -1,7 +1,7 @@
 name = "nodejs-hybrid-app"
 main = "src/index.ts"
 compatibility_date = "2024-06-03"
-compatibility_flags = ["experimental:nodejs_compat_v2"]
+compatibility_flags = ["nodejs_compat_v2"]
 
 [vars]
 # These DB connection values are to a public database containing information about

fixtures/pages-nodejs-v2-compat/wrangler.toml

@@ -1,3 +1,3 @@
 name = "pages-nodejs-compat"
 compatibility_date = "2024-08-20"
-compatibility_flags = ["experimental:nodejs_compat_v2"]
+compatibility_flags = ["nodejs_compat_v2"]

packages/wrangler/src/__tests__/deploy.test.ts

@@ -9930,10 +9930,10 @@ export default{
 				"deploy index.js --no-bundle --node-compat --dry-run --outdir dist"
 			);
 			expect(std.warn).toMatchInlineSnapshot(`
-				"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.[0m
+				"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1m\`--node-compat\` 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.[0m
 
 
-				[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1m\`--node-compat\` 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.[0m
+				[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.[0m
 
 				"
 			`);
@@ -9950,10 +9950,10 @@ export default{
 			fs.writeFileSync("index.js", scriptContent);
 			await runWrangler("deploy index.js --dry-run --outdir dist");
 			expect(std.warn).toMatchInlineSnapshot(`
-				"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.[0m
+				"[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1m\`--node-compat\` 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.[0m
 
 
-				[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1m\`--node-compat\` 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.[0m
+				[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs.[0m
 
 				"
 			`);

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,12 +174,13 @@ export async function deploy({
 		}
 	}
 
-	const nodejsCompatMode = validateNodeCompat({
-		legacyNodeCompat: false,
-		compatibilityFlags:
-			config?.compatibility_flags ?? deploymentConfig.compatibility_flags ?? [],
-		noBundle: config?.no_bundle ?? false,
-	});
+	const nodejsCompatMode = getNodeCompatMode(
+		config?.compatibility_flags ?? deploymentConfig.compatibility_flags ?? [],
+		{
+			nodeCompat: false,
+			noBundle: config?.no_bundle,
+		}
+	);
 	const defineNavigatorUserAgent = isNavigatorDefined(
 		config?.compatibility_date ?? deploymentConfig.compatibility_date,
 		config?.compatibility_flags ?? deploymentConfig.compatibility_flags

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,10 +399,9 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m
 
 	const compatibilityFlags =
 		props.compatibilityFlags ?? config.compatibility_flags;
-	const nodejsCompatMode = validateNodeCompat({
-		legacyNodeCompat: props.nodeCompat ?? config.node_compat ?? false,
-		compatibilityFlags,
-		noBundle: props.noBundle ?? config.no_bundle ?? false,
+	const nodejsCompatMode = getNodeCompatMode(compatibilityFlags, {
+		nodeCompat: props.nodeCompat ?? config.node_compat,
+		noBundle: props.noBundle ?? config.no_bundle,
 	});
 
 	// Warn if user tries minify with no-bundle

packages/wrangler/src/deployment-bundle/create-worker-upload-form.ts

@@ -3,7 +3,6 @@ import { readFileSync } from "node:fs";
 import path from "node:path";
 import { File, FormData } from "undici";
 import { handleUnsafeCapnp } from "./capnp";
-import { stripExperimentalPrefixes } from "./node-compat";
 import type {
 	CfDurableObjectMigrations,
 	CfModuleType,
@@ -547,7 +546,7 @@ export function createWorkerUploadForm(worker: CfWorkerInit): FormData {
 		bindings: metadataBindings,
 		...(compatibility_date && { compatibility_date }),
 		...(compatibility_flags && {
-			compatibility_flags: stripExperimentalPrefixes(compatibility_flags),
+			compatibility_flags,
 		}),
 		...(migrations && { migrations }),
 		capnp_schema: capnpSchemaOutputFile,

packages/wrangler/src/deployment-bundle/node-compat.ts

@@ -1,6 +1,5 @@
 import { UserError } from "../errors";
 import { logger } from "../logger";
-import type { Config } from "../config";
 
 /**
  * Wrangler can provide Node.js compatibility in a number of different modes:
@@ -12,121 +11,102 @@ import type { Config } from "../config";
 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": experimental nodejs_compat_v2 flag
+ *  - "v2": nodejs_compat_v2 compatibility flag
  *  - null: no Node.js compatibility
- *
- * Currently we require that the v2 mode is configured via `experimental:nodejs_compat_v2` compat flag,
- * where the `experimental:` prefix is stripped before being passed to the runtime since that does not
- * understand this prefix.
- *
- * 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({
-	legacyNodeCompat,
-	compatibilityFlags,
-	noBundle,
-}: {
-	legacyNodeCompat: boolean;
-	/* mutate */ compatibilityFlags: string[];
-	noBundle: boolean;
-}): NodeJSCompatMode {
-	if (legacyNodeCompat) {
-		logger.warn(
-			"Enabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs."
-		);
+export function getNodeCompatMode(
+	compatibilityFlags: string[],
+	{
+		validateConfig = true,
+		nodeCompat = undefined,
+		noBundle = undefined,
+	}: {
+		validateConfig?: boolean;
+		nodeCompat?: boolean;
+		noBundle?: boolean;
 	}
+): NodeJSCompatMode {
+	const {
+		hasNodejsCompatFlag,
+		hasNodejsCompatV2Flag,
+		hasExperimentalNodejsCompatV2Flag,
+	} = parseNodeCompatibilityFlags(compatibilityFlags);
 
-	const { mode, nodejsCompat, nodejsCompatV2 } = getNodeCompatMode({
-		compatibility_flags: compatibilityFlags,
-		node_compat: legacyNodeCompat,
-	});
+	const legacy = nodeCompat === true;
+	let mode: NodeJSCompatMode = null;
+	if (hasNodejsCompatV2Flag) {
+		mode = "v2";
+	} else if (hasNodejsCompatFlag) {
+		mode = "v1";
+	} else if (legacy) {
+		mode = "legacy";
+	}
 
-	const nodejsCompatV2NotExperimental =
-		compatibilityFlags.includes("nodejs_compat_v2");
+	if (validateConfig !== true) {
+		// Skip the validation.
+		return mode;
+	}
 
-	if (nodejsCompat && nodejsCompatV2) {
+	if (hasExperimentalNodejsCompatV2Flag) {
 		throw new UserError(
-			"The `nodejs_compat` and `nodejs_compat_v2` compatibility flags cannot be used in together. Please select just one."
+			"The `experimental:` prefix on `nodejs_compat_v2` is no longer valid. Please remove it and try again."
 		);
 	}
 
-	if (legacyNodeCompat && (nodejsCompat || nodejsCompatV2)) {
+	if (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 `nodejs_compat` and `nodejs_compat_v2` compatibility flags cannot be used in together. Please select just one."
 		);
 	}
 
-	if (nodejsCompatV2NotExperimental) {
+	if (legacy && (hasNodejsCompatFlag || hasNodejsCompatV2Flag)) {
 		throw new UserError(
-			`The \`nodejs_compat_v2\` compatibility flag is experimental and must be prefixed with \`experimental:\`. Use \`experimental:nodejs_compat_v2\` flag instead.`
+			`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.`
 		);
 	}
 
-	if (nodejsCompatV2) {
+	if (noBundle && legacy) {
 		logger.warn(
-			"Enabling experimental Node.js compatibility mode v2. This feature is still in development and not ready for production use."
+			"`--node-compat` 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."
 		);
 	}
 
-	if (noBundle && legacyNodeCompat) {
+	if (noBundle && hasNodejsCompatV2Flag) {
 		logger.warn(
-			"`--node-compat` 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."
+			"`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."
 		);
 	}
 
-	if (noBundle && nodejsCompatV2) {
+	if (mode === "legacy") {
 		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."
+			"Enabling Wrangler compile-time Node.js compatibility polyfill mode for builtins and globals. This is experimental and has serious tradeoffs."
 		);
 	}
 
 	return mode;
 }
 
-export function getNodeCompatMode({
-	compatibility_flags,
-	node_compat,
-}: Pick<Config, "compatibility_flags" | "node_compat">) {
-	const nodejsCompat = compatibility_flags.includes("nodejs_compat");
-	const nodejsCompatV2 = compatibility_flags.includes(
-		"experimental:nodejs_compat_v2"
-	);
-
-	let mode: NodeJSCompatMode;
-	if (nodejsCompatV2) {
-		mode = "v2";
-	} else if (nodejsCompat) {
-		mode = "v1";
-	} else if (node_compat) {
-		mode = "legacy";
-	} else {
-		mode = null;
-	}
-
+function parseNodeCompatibilityFlags(compatibilityFlags: string[]) {
 	return {
-		legacy: node_compat === true,
-		mode,
-		nodejsCompat,
-		nodejsCompatV2,
+		hasNodejsCompatFlag: compatibilityFlags.includes("nodejs_compat"),
+		hasNodejsCompatV2Flag: compatibilityFlags.includes("nodejs_compat_v2"),
+		hasExperimentalNodejsCompatV2Flag: compatibilityFlags.includes(
+			"experimental:nodejs_compat_v2"
+		),
 	};
 }
-
-/**
- * The nodejs_compat_v2 flag currently requires an `experimental:` prefix within Wrangler,
- * but this needs to be stripped before sending to workerd, since that doesn't know about that.
- *
- * TODO: Remove this function when we graduate nodejs_v2 to non-experimental.
- * See https://jira.cfdata.org/browse/DEVDASH-218
- */
-export function stripExperimentalPrefixes(
-	compatFlags: string[] | undefined
-): string[] | undefined {
-	return compatFlags?.map((flag) => flag.replace(/^experimental:/, ""));
-}

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,15 +680,13 @@ export async function startDev(args: StartDevOptions) {
 					moduleRoot: args.moduleRoot,
 					moduleRules: args.rules,
 					nodejsCompatMode: (parsedConfig: Config) =>
-						validateNodeCompat({
-							legacyNodeCompat:
-								args.nodeCompat ?? parsedConfig.node_compat ?? false,
-							compatibilityFlags:
-								args.compatibilityFlags ??
-								parsedConfig.compatibility_flags ??
-								[],
-							noBundle: args.noBundle ?? parsedConfig.no_bundle ?? false,
-						}),
+						getNodeCompatMode(
+							args.compatibilityFlags ?? parsedConfig.compatibility_flags ?? [],
+							{
+								nodeCompat: args.nodeCompat ?? parsedConfig.node_compat,
+								noBundle: args.noBundle ?? parsedConfig.no_bundle,
+							}
+						),
 				},
 				bindings: {
 					...(await getPagesAssetsFetcher(
@@ -833,12 +831,13 @@ export async function startDev(args: StartDevOptions) {
 			additionalModules,
 		} = await validateDevServerSettings(args, config);
 
-		const nodejsCompatMode = validateNodeCompat({
-			legacyNodeCompat: args.nodeCompat ?? config.node_compat ?? false,
-			compatibilityFlags:
-				args.compatibilityFlags ?? config.compatibility_flags ?? [],
-			noBundle: args.noBundle ?? config.no_bundle ?? false,
-		});
+		const nodejsCompatMode = getNodeCompatMode(
+			args.compatibilityFlags ?? config.compatibility_flags ?? [],
+			{
+				nodeCompat: args.nodeCompat ?? config.node_compat,
+				noBundle: args.noBundle ?? config.no_bundle,
+			}
+		);
 
 		void metrics.sendMetricsEvent(
 			"run dev",
@@ -973,11 +972,13 @@ export async function startApiDev(args: StartDevOptions) {
 		additionalModules,
 	} = await validateDevServerSettings(args, config);
 
-	const nodejsCompatMode = validateNodeCompat({
-		legacyNodeCompat: args.nodeCompat ?? config.node_compat ?? false,
-		compatibilityFlags: args.compatibilityFlags ?? config.compatibility_flags,
-		noBundle: args.noBundle ?? config.no_bundle ?? false,
-	});
+	const nodejsCompatMode = getNodeCompatMode(
+		args.compatibilityFlags ?? config.compatibility_flags,
+		{
+			nodeCompat: args.nodeCompat ?? config.node_compat,
+			noBundle: args.noBundle ?? config.no_bundle,
+		}
+	);
 
 	await metrics.sendMetricsEvent(
 		"run dev (api)",

packages/wrangler/src/dev/miniflare.ts

@@ -18,7 +18,6 @@ import {
 } from "../ai/fetcher";
 import { readConfig } from "../config";
 import { ModuleTypeToRuleType } from "../deployment-bundle/module-collection";
-import { stripExperimentalPrefixes } from "../deployment-bundle/node-compat";
 import { withSourceURLs } from "../deployment-bundle/source-url";
 import { UserError } from "../errors";
 import { logger } from "../logger";
@@ -894,9 +893,7 @@ export async function buildMiniflareOptions(
 			{
 				name: getName(config),
 				compatibilityDate: config.compatibilityDate,
-				compatibilityFlags: stripExperimentalPrefixes(
-					config.compatibilityFlags
-				),
+				compatibilityFlags: config.compatibilityFlags,
 
 				...sourceOptions,
 				...bindingOptions,