Fix nodejs compat v2 upgrade (#6824)

* fix: tidy up error messaging for unexpected use of Node.js APIs

Fixes #6822

* fix: teach Wrangler about node_compat version date switch

A compatibility of Sept 23, 2024 or later means that `nodejs_compat` is equivalent to
`nodejs_compat_v2`.

* refactor: move the node-compat parsing from Wrangler into Miniflare

This logic is now also needed inside Miniflare to determine how to handle node imports. So this commit moves the relevant code in there and references it from Wrangler.

The validation of the config is kept in Wrangler, since Miniflare doesn't need that
particularly and the validation relies upon Wrangler logger and UserError.

* fix: teach Miniflare about node_compat version date switch

A compatibility of Sept 23, 2024 or later means that `nodejs_compat` is equivalent to `nodejs_compat_v2`.

* test: update Node.js hybrid fixture to use nodejs_compat + compat date

Now that we can set the compat date to Sept 23, 2024, we can update this fixture not to use the `nodejs_compat_v2` flag.

* fixup! fix: teach Miniflare about node_compat version date switch

* fixup! refactor: move the node-compat parsing from Wrangler into Miniflare

* fixup! fix: tidy up error messaging for unexpected use of Node.js APIs

* fixup! fix: tidy up error messaging for unexpected use of Node.js APIs

* fixup! refactor: move the node-compat parsing from Wrangler into Miniflare

* fixup! fix: tidy up error messaging for unexpected use of Node.js APIs

* fixup! refactor: move the node-compat parsing from Wrangler into Miniflare

* fix: also handle the case with `no_nodejs_compat_v2`

* test: normalize output strings for tests

* refactor: rename `getNodejsCompatMode()` to `getNodejsCompat()`

.changeset/early-tigers-buy.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: tidy up error messaging for unexpected use of Node.js APIs
+
+Fixes #6822

.changeset/tender-items-exercise.md

@@ -0,0 +1,7 @@
+---
+"miniflare": patch
+---
+
+fix: teach Miniflare about node_compat version date switch
+
+A compatibility of Sept 23, 2024 or later means that `nodejs_compat` is equivalent to `nodejs_compat_v2`.

fixtures/nodejs-hybrid-app/wrangler.toml

@@ -1,7 +1,8 @@
 name = "nodejs-hybrid-app"
 main = "src/index.ts"
-compatibility_date = "2024-06-03"
-compatibility_flags = ["nodejs_compat_v2"]
+# Setting compat date to 2024/09/23 means we don't need to use `nodejs_compat_v2`
+compatibility_date = "2024-09-23"
+compatibility_flags = ["nodejs_compat"]
 
 [vars]
 # These DB connection values are to a public database containing information about

packages/miniflare/src/plugins/core/index.ts

@@ -807,7 +807,10 @@ export function getGlobalServices({
 }
 
 function getWorkerScript(
-	options: SourceOptions & { compatibilityFlags?: string[] },
+	options: SourceOptions & {
+		compatibilityDate?: string;
+		compatibilityFlags?: string[];
+	},
 	workerIndex: number,
 	additionalModuleNames: string[]
 ): { serviceWorkerScript: string } | { modules: Worker_Module[] } {
@@ -843,6 +846,7 @@ function getWorkerScript(
 			modulesRoot,
 			additionalModuleNames,
 			options.modulesRules,
+			options.compatibilityDate,
 			options.compatibilityFlags
 		);
 		// If `script` and `scriptPath` are set, resolve modules in `script`
@@ -862,3 +866,4 @@ export * from "./proxy";
 export * from "./constants";
 export * from "./modules";
 export * from "./services";
+export * from "./node-compat";

packages/miniflare/src/plugins/core/modules.ts

@@ -11,6 +11,7 @@ import { z } from "zod";
 import { Worker_Module } from "../../runtime";
 import { globsToRegExps, MiniflareCoreError, PathSchema } from "../../shared";
 import { MatcherRegExps, testRegExps } from "../../workers";
+import { getNodeCompat, NodeJSCompatMode } from "./node-compat";
 import type estree from "estree";
 
 const SUGGEST_BUNDLE =
@@ -156,23 +157,24 @@ function getResolveErrorPrefix(referencingPath: string): string {
 
 export class ModuleLocator {
 	readonly #compiledRules: CompiledModuleRule[];
-	readonly #nodejsCompat: boolean;
+	readonly #nodejsCompatMode: NodeJSCompatMode;
 	readonly #visitedPaths = new Set<string>();
 	readonly modules: Worker_Module[] = [];
 
 	constructor(
 		private readonly modulesRoot: string,
 		private readonly additionalModuleNames: string[],
 		rules: ModuleRule[] = [],
+		compatibilityDate?: string,
 		compatibilityFlags?: string[]
 	) {
 		// Implicit shallow-copy to avoid mutating argument
 		rules = rules.concat(DEFAULT_MODULE_RULES);
 		this.#compiledRules = compileModuleRules(rules);
-		// `nodejs_compat` doesn't have a default-on date, so we know whether it's
-		// enabled just by looking at flags:
-		// https://github.com/cloudflare/workerd/blob/edcd0300bc7b8f56040d090177db947edd22f91b/src/workerd/io/compatibility-date.capnp#L237-L240
-		this.#nodejsCompat = compatibilityFlags?.includes("nodejs_compat") ?? false;
+		this.#nodejsCompatMode = getNodeCompat(
+			compatibilityDate,
+			compatibilityFlags ?? []
+		).mode;
 	}
 
 	visitEntrypoint(code: string, modulePath: string) {
@@ -289,14 +291,19 @@ ${dim(modulesConfig)}`;
 		}
 		const spec = specExpression.value;
 
-		// `node:` (assuming `nodejs_compat` flag enabled), `cloudflare:` and
-		// `workerd:` imports don't need to be included explicitly
 		const isNodeJsCompatModule = referencingType === "NodeJsCompatModule";
 		if (
-			(this.#nodejsCompat && spec.startsWith("node:")) ||
+			// `cloudflare:` and `workerd:` imports don't need to be included explicitly
 			spec.startsWith("cloudflare:") ||
 			spec.startsWith("workerd:") ||
-			(isNodeJsCompatModule && builtinModulesWithPrefix.includes(spec)) ||
+			// Node.js compat v1 requires imports to be prefixed with `node:`
+			(this.#nodejsCompatMode === "v1" && spec.startsWith("node:")) ||
+			// Node.js compat modules and v2 can also handle non-prefixed imports
+			((this.#nodejsCompatMode === "v2" || isNodeJsCompatModule) &&
+				builtinModulesWithPrefix.includes(spec)) ||
+			// Async Local Storage mode (node_als) only deals with `node:async_hooks` imports
+			(this.#nodejsCompatMode === "als" && spec === "node:async_hooks") ||
+			// Any "additional" external modules can be ignored
 			this.additionalModuleNames.includes(spec)
 		) {
 			return;

packages/miniflare/src/plugins/core/node-compat.ts

@@ -0,0 +1,78 @@
+/**
+ * We can provide Node.js compatibility in a number of different modes:
+ * - "legacy" - this mode adds compile-time polyfills that are not well maintained and cannot work with workerd runtime builtins.
+ * - "als": this mode tells the workerd runtime to enable only the Async Local Storage builtin library (accessible via `node:async_hooks`).
+ * - "v1" - this mode tells the workerd runtime to enable some Node.js builtin libraries (accessible only via `node:...` imports) but no globals.
+ * - "v2" - this mode tells the workerd runtime to enable more Node.js builtin libraries (accessible both with and without the `node:` prefix)
+ *   and also some Node.js globals such as `Buffer`; it also turns on additional compile-time polyfills for those that are not provided by the runtime.
+ *  - null - no Node.js compatibility.
+ */
+export type NodeJSCompatMode = "legacy" | "als" | "v1" | "v2" | null;
+
+/**
+ * Computes the Node.js compatibility mode we are running.
+ *
+ * NOTES:
+ * - The v2 mode is configured via `nodejs_compat_v2` compat flag or via `nodejs_compat` plus a compatibility date of Sept 23rd. 2024 or later.
+ * - See `EnvironmentInheritable` for `nodeCompat` and `noBundle`.
+ *
+ * @param compatibilityDateStr The compatibility date
+ * @param compatibilityFlags The compatibility flags
+ * @param opts.nodeCompat Whether the legacy node_compat arg is being used
+ * @returns the mode and flags to indicate specific configuration for validating.
+ */
+export function getNodeCompat(
+	compatibilityDate: string = "2000-01-01", // Default to some arbitrary old date
+	compatibilityFlags: string[],
+	opts?: {
+		nodeCompat?: boolean;
+	}
+) {
+	const { nodeCompat = false } = opts ?? {};
+	const {
+		hasNodejsAlsFlag,
+		hasNodejsCompatFlag,
+		hasNodejsCompatV2Flag,
+		hasNoNodejsCompatV2Flag,
+		hasExperimentalNodejsCompatV2Flag,
+	} = parseNodeCompatibilityFlags(compatibilityFlags);
+
+	const nodeCompatSwitchOverDate = "2024-09-23";
+	const legacy = nodeCompat === true;
+	let mode: NodeJSCompatMode = null;
+	if (
+		hasNodejsCompatV2Flag ||
+		(hasNodejsCompatFlag &&
+			compatibilityDate >= nodeCompatSwitchOverDate &&
+			!hasNoNodejsCompatV2Flag)
+	) {
+		mode = "v2";
+	} else if (hasNodejsCompatFlag) {
+		mode = "v1";
+	} else if (hasNodejsAlsFlag) {
+		mode = "als";
+	} else if (legacy) {
+		mode = "legacy";
+	}
+
+	return {
+		mode,
+		hasNodejsAlsFlag,
+		hasNodejsCompatFlag,
+		hasNodejsCompatV2Flag,
+		hasNoNodejsCompatV2Flag,
+		hasExperimentalNodejsCompatV2Flag,
+	};
+}
+
+function parseNodeCompatibilityFlags(compatibilityFlags: string[]) {
+	return {
+		hasNodejsAlsFlag: compatibilityFlags.includes("nodejs_als"),
+		hasNodejsCompatFlag: compatibilityFlags.includes("nodejs_compat"),
+		hasNodejsCompatV2Flag: compatibilityFlags.includes("nodejs_compat_v2"),
+		hasNoNodejsCompatV2Flag: compatibilityFlags.includes("no_nodejs_compat_v2"),
+		hasExperimentalNodejsCompatV2Flag: compatibilityFlags.includes(
+			"experimental:nodejs_compat_v2"
+		),
+	};
+}

packages/miniflare/src/plugins/index.ts

@@ -103,6 +103,7 @@ export {
 	ProxyClient,
 	getFreshSourceMapSupport,
 	kCurrentWorker,
+	getNodeCompat,
 } from "./core";
 export type {
 	CompiledModuleRule,
@@ -111,6 +112,7 @@ export type {
 	ModuleDefinition,
 	GlobalServicesOptions,
 	SourceOptions,
+	NodeJSCompatMode,
 } from "./core";
 export type * from "./core/proxy/types";
 export * from "./d1";

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

@@ -1,7 +1,7 @@
 import path from "node:path";
 import { readConfig } from "../config";
 import { normalizeAndValidateConfig } from "../config/validation";
-import { normalizeSlashes } from "./helpers/mock-console";
+import { normalizeString } from "./helpers/normalize";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import { writeWranglerToml } from "./helpers/write-wrangler-toml";
 import type {
@@ -301,7 +301,7 @@ describe("normalizeAndValidateConfig()", () => {
 				expect(diagnostics.hasErrors()).toBe(false);
 				expect(diagnostics.hasWarnings()).toBe(true);
 
-				expect(normalizeSlashes(diagnostics.renderWarnings()))
+				expect(normalizeString(diagnostics.renderWarnings()))
 					.toMatchInlineSnapshot(`
 			          "Processing wrangler configuration:
 			            - Deprecation: \\"site.entry-point\\":
@@ -332,7 +332,7 @@ describe("normalizeAndValidateConfig()", () => {
 				expect(diagnostics.hasWarnings()).toBe(true);
 				expect(diagnostics.hasErrors()).toBe(true);
 
-				expect(normalizeSlashes(diagnostics.renderWarnings()))
+				expect(normalizeString(diagnostics.renderWarnings()))
 					.toMatchInlineSnapshot(`
 			          "Processing wrangler configuration:
 			            - Deprecation: \\"site.entry-point\\":
@@ -375,7 +375,7 @@ describe("normalizeAndValidateConfig()", () => {
 			            - Expected \\"site.entry-point\\" to be of type string but got 111."
 		        `);
 
-				expect(normalizeSlashes(diagnostics.renderWarnings()))
+				expect(normalizeString(diagnostics.renderWarnings()))
 					.toMatchInlineSnapshot(`
 			          "Processing wrangler configuration:
 			            - Deprecation: \\"site.entry-point\\":
@@ -409,7 +409,7 @@ describe("normalizeAndValidateConfig()", () => {
 				expect(diagnostics.hasWarnings()).toBe(true);
 				expect(diagnostics.hasErrors()).toBe(false);
 
-				expect(normalizeSlashes(diagnostics.renderWarnings()))
+				expect(normalizeString(diagnostics.renderWarnings()))
 					.toMatchInlineSnapshot(`
 			          "Processing wrangler configuration:
 			            - Deprecation: \\"site.entry-point\\":
@@ -881,7 +881,7 @@ describe("normalizeAndValidateConfig()", () => {
 			expect(diagnostics.hasErrors()).toBe(false);
 			expect(diagnostics.hasWarnings()).toBe(true);
 
-			expect(normalizeSlashes(diagnostics.renderWarnings()))
+			expect(normalizeString(diagnostics.renderWarnings()))
 				.toMatchInlineSnapshot(`
 "Processing project/wrangler.toml configuration:
   - \\"unsafe\\" fields are experimental and may change or break at any time.