Remove NodeJSCompatModule (#8666)

Author: penalosa

.changeset/violet-ravens-draw.md

@@ -0,0 +1,7 @@
+---
+"miniflare": patch
+"@cloudflare/vitest-pool-workers": patch
+"wrangler": patch
+---
+
+Remove `NodeJSCompatModule`. This was never fully supported, and never worked for deploying Workers from Wrangler.

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

@@ -21,7 +21,7 @@ const SUGGEST_NODE =
 	"that uses Node.js built-ins, you'll either need to:" +
 	"\n- Bundle your Worker, configuring your bundler to polyfill Node.js built-ins" +
 	"\n- Configure your bundler to load Workers-compatible builds by changing the main fields/conditions" +
-	"\n- Enable the `nodejs_compat` compatibility flag and use the `NodeJsCompatModule` module type" +
+	"\n- Enable the `nodejs_compat` compatibility flag" +
 	"\n- Find an alternative package that doesn't require Node.js built-ins";
 
 const builtinModulesWithPrefix = builtinModules.concat(
@@ -43,7 +43,6 @@ export function maybeGetStringScriptPathIndex(
 export const ModuleRuleTypeSchema = z.enum([
 	"ESModule",
 	"CommonJS",
-	"NodeJsCompatModule",
 	"Text",
 	"Data",
 	"CompiledWasm",
@@ -52,7 +51,7 @@ export const ModuleRuleTypeSchema = z.enum([
 ]);
 export type ModuleRuleType = z.infer<typeof ModuleRuleTypeSchema>;
 
-type JavaScriptModuleRuleType = "ESModule" | "CommonJS" | "NodeJsCompatModule";
+type JavaScriptModuleRuleType = "ESModule" | "CommonJS";
 
 export const ModuleRuleSchema = z.object({
 	type: ModuleRuleTypeSchema,
@@ -291,15 +290,14 @@ ${dim(modulesConfig)}`;
 		}
 		const spec = specExpression.value;
 
-		const isNodeJsCompatModule = referencingType === "NodeJsCompatModule";
 		if (
 			// `cloudflare:` and `workerd:` imports don't need to be included explicitly
 			spec.startsWith("cloudflare:") ||
 			spec.startsWith("workerd:") ||
 			// 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) &&
+			(this.#nodejsCompatMode === "v2" &&
 				builtinModulesWithPrefix.includes(spec)) ||
 			// Async Local Storage mode (node_als) only deals with `node:async_hooks` imports
 			(this.#nodejsCompatMode === "als" && spec === "node:async_hooks") ||
@@ -345,7 +343,6 @@ ${dim(modulesConfig)}`;
 		switch (rule.type) {
 			case "ESModule":
 			case "CommonJS":
-			case "NodeJsCompatModule":
 				const code = data.toString("utf8");
 				this.#visitJavaScriptModule(code, identifier, rule.type);
 				break;
@@ -383,8 +380,6 @@ function createJavaScriptModule(
 		return { name, esModule: code };
 	} else if (type === "CommonJS") {
 		return { name, commonJsModule: code };
-	} else if (type === "NodeJsCompatModule") {
-		return { name, nodeJsCompatModule: code };
 	}
 	// noinspection UnnecessaryLocalVariableJS
 	const exhaustive: never = type;
@@ -409,7 +404,6 @@ export function convertModuleDefinition(
 	switch (def.type) {
 		case "ESModule":
 		case "CommonJS":
-		case "NodeJsCompatModule":
 			return createJavaScriptModule(
 				contentsToString(contents),
 				name,
@@ -441,8 +435,6 @@ function convertWorkerModule(mod: Worker_Module): ModuleDefinition {
 
 	if ("esModule" in m) return { path, type: "ESModule" };
 	else if ("commonJsModule" in m) return { path, type: "CommonJS" };
-	else if ("nodeJsCompatModule" in m)
-		return { path, type: "NodeJsCompatModule" };
 	else if ("text" in m) return { path, type: "Text" };
 	else if ("data" in m) return { path, type: "Data" };
 	else if ("wasm" in m) return { path, type: "CompiledWasm" };

packages/miniflare/src/runtime/config/workerd.ts

@@ -78,7 +78,6 @@ export type Worker_Module = {
 	| { data?: Uint8Array }
 	| { wasm?: Uint8Array }
 	| { json?: string }
-	| { nodeJsCompatModule?: string }
 	| { pythonModule?: string }
 	| { pythonRequirement?: string }
 );

packages/miniflare/test/fixtures/modules/index.node.cjs

@@ -3,7 +3,7 @@ const assert = require("assert");
 // Test `require()` of Node built-in module with prefix
 const assert2 = require("node:assert");
 
-// `Buffer` should be global in `NodeJsCompatModule`s
+// `Buffer` should be global in `CommonJS`s with node_compat_v2 turned on
 assert.strictEqual(typeof Buffer, "function");
 assert2(true);
 

packages/miniflare/test/plugins/core/modules.spec.ts

@@ -20,7 +20,7 @@ test("Miniflare: accepts manually defined modules", async (t) => {
 	// Check with just `path`
 	const mf = new Miniflare({
 		compatibilityDate: "2023-08-01",
-		compatibilityFlags: ["nodejs_compat"],
+		compatibilityFlags: ["nodejs_compat_v2"],
 		// TODO(soon): remove `modulesRoot` once https://github.com/cloudflare/workerd/issues/1101 fixed
 		//  and add separate test for that
 		modulesRoot: ROOT,
@@ -29,7 +29,7 @@ test("Miniflare: accepts manually defined modules", async (t) => {
 			{ type: "ESModule", path: path.join(ROOT, "blobs.mjs") },
 			{ type: "ESModule", path: path.join(ROOT, "blobs-indirect.mjs") },
 			{ type: "CommonJS", path: path.join(ROOT, "index.cjs") },
-			{ type: "NodeJsCompatModule", path: path.join(ROOT, "index.node.cjs") },
+			{ type: "CommonJS", path: path.join(ROOT, "index.node.cjs") },
 			// Testing modules in subdirectories
 			{ type: "Text", path: path.join(ROOT, "blobs", "text.txt") },
 			{ type: "Data", path: path.join(ROOT, "blobs", "data.bin") },
@@ -51,7 +51,7 @@ test("Miniflare: accepts manually defined modules", async (t) => {
 		"AGFzbQEAAAABBwFgAn9/AX8DAgEABwcBA2FkZAAACgkBBwAgACABawsACgRuYW1lAgMBAAA=";
 	await mf.setOptions({
 		compatibilityDate: "2023-08-01",
-		compatibilityFlags: ["nodejs_compat"],
+		compatibilityFlags: ["nodejs_compat_v2"],
 		modules: [
 			{ type: "ESModule", path: path.join(ROOT, "index.mjs") },
 			{
@@ -79,7 +79,7 @@ test("Miniflare: accepts manually defined modules", async (t) => {
         `,
 			},
 			{
-				type: "NodeJsCompatModule",
+				type: "CommonJS",
 				path: path.join(ROOT, "index.node.cjs"),
 				contents: `module.exports = "node:";`,
 			},
@@ -112,14 +112,14 @@ test("Miniflare: automatically collects modules", async (t) => {
 		modules: true,
 		modulesRoot: ROOT,
 		modulesRules: [
-			// Implicitly testing default module rules for `ESModule` and `CommonJS`
-			{ type: "NodeJsCompatModule", include: ["**/*.node.cjs"] },
+			// Implicitly testing default module rules for `ESModule`
+			{ type: "CommonJS", include: ["**/*.node.cjs", "**/*.cjs"] },
 			{ type: "Text", include: ["**/*.txt"] },
 			{ type: "Data", include: ["**/*.bin"] },
 			{ type: "CompiledWasm", include: ["**/*.wasm"] },
 		],
 		compatibilityDate: "2023-08-01",
-		compatibilityFlags: ["nodejs_compat"],
+		compatibilityFlags: ["nodejs_compat_v2"],
 		scriptPath: path.join(ROOT, "index.mjs"),
 	});
 	t.teardown(() => mf.dispose());
@@ -206,14 +206,14 @@ test("Miniflare: cannot automatically collect modules from dynamic import expres
 		modules: true,
 		modulesRoot: ROOT,
 		modulesRules: [
-			// Implicitly testing default module rules for `ESModule` and `CommonJS`
-			{ type: "NodeJsCompatModule", include: ["**/*.node.cjs"] },
+			// Implicitly testing default module rules for `ESModule`
+			{ type: "CommonJS", include: ["**/*.node.cjs", "**/*.cjs"] },
 			{ type: "Text", include: ["**/*.txt"] },
 			{ type: "Data", include: ["**/*.bin"] },
 			{ type: "CompiledWasm", include: ["**/*.wasm"] },
 		],
 		compatibilityDate: "2023-08-01",
-		compatibilityFlags: ["nodejs_compat"],
+		compatibilityFlags: ["nodejs_compat_v2"],
 		scriptPath,
 	});
 
@@ -233,7 +233,7 @@ You must manually define your modules when constructing Miniflare:
     modules: [
       { type: "ESModule", path: "index-dynamic.mjs" },
       { type: "CommonJS", path: "index.cjs" },
-      { type: "NodeJsCompatModule", path: "index.node.cjs" },
+      { type: "CommonJS", path: "index.node.cjs" },
       { type: "ESModule", path: "blobs-indirect.mjs" },
       { type: "ESModule", path: "blobs.mjs" },
       { type: "Text", path: "blobs/text.txt" },

packages/vitest-pool-workers/src/pool/index.ts

@@ -372,11 +372,8 @@ function buildProjectWorkerOptions(
 		runnerWorker.compatibilityFlags
 	);
 
-	if (mode !== "v1" && mode !== "v2") {
-		runnerWorker.compatibilityFlags.push(
-			"nodejs_compat",
-			"no_nodejs_compat_v2"
-		);
+	if (mode !== "v2") {
+		runnerWorker.compatibilityFlags.push("nodejs_compat_v2");
 	}
 
 	// Required for `workerd:unsafe` module. We don't require this flag to be set

packages/vitest-pool-workers/src/pool/module-fallback.ts

@@ -409,8 +409,6 @@ function maybeGetForceTypeModuleContents(
 			return { esModule: contents.toString() };
 		case "CommonJS":
 			return { commonJsModule: contents.toString() };
-		case "NodeJsCompatModule":
-			return { nodeJsCompatModule: contents.toString() };
 		case "Text":
 			return { text: contents.toString() };
 		case "Data":
@@ -510,8 +508,8 @@ async function load(
 	// Respond with CommonJS module
 
 	// If we're `import`ing a CommonJS module, or we're `require`ing a `node:*`
-	// module from a NodeJsCompatModule, return an ES module shim. Note
-	// NodeJsCompatModules can `require` ES modules, using the default export.
+	// module from a CommonJS, return an ES module shim. Note
+	// CommonJS can `require` ES modules, using the default export.
 	const insertCjsEsmShim = method === "import" || specifier.startsWith("node:");
 	if (insertCjsEsmShim && !disableCjsEsmShim) {
 		const fileName = posixPath.basename(filePath);
@@ -526,9 +524,9 @@ async function load(
 	}
 
 	// Otherwise, if we're `require`ing a non-`node:*` module, just return a
-	// NodeJsCompatModule
+	// CommonJS
 	debuglog(logBase, "cjs:", filePath);
-	return buildModuleResponse(target, { nodeJsCompatModule: contents });
+	return buildModuleResponse(target, { commonJsModule: contents });
 }
 
 export async function handleModuleFallbackRequest(

packages/wrangler/src/__tests__/api/startDevWorker/LocalRuntimeController.test.ts

@@ -149,7 +149,7 @@ describe("LocalRuntimeController", () => {
 			const config = {
 				name: "worker",
 				entrypoint: "NOT_REAL",
-				compatibilityFlags: ["nodejs_compat"],
+				compatibilityFlags: ["nodejs_compat_v2"],
 				compatibilityDate: "2023-10-01",
 			};
 			const bundle: Bundle = {
@@ -171,7 +171,7 @@ describe("LocalRuntimeController", () => {
 					`,
 					},
 					{
-						type: "nodejs-compat-module",
+						type: "commonjs",
 						name: "base64.cjs",
 						filePath: "/virtual/node/base64.cjs",
 						content: `module.exports = {
@@ -223,7 +223,7 @@ describe("LocalRuntimeController", () => {
 							});
 						} else if (pathname === "/throw-commonjs") {
 							try { add.throw(); } catch (e) { return new Response(e.stack); }
-						} else if (pathname === "/throw-nodejs-compat-module") {
+						} else if (pathname === "/throw-other-commonjs") {
 							try { base64.throw(); } catch (e) { return new Response(e.stack); }
 						} else {
 							return new Response(null, { status: 404 });
@@ -270,12 +270,12 @@ describe("LocalRuntimeController", () => {
 			    at Object.fetch (file:///D:/virtual/esm/index.mjs:15:19)"
 			`);
 
-				// Check stack traces from NodeJsCompatModule modules include file path
-				res = await fetch(new URL("/throw-nodejs-compat-module", url));
+				// Check stack traces from CommonJS modules include file path
+				res = await fetch(new URL("/throw-other-commonjs", url));
 				expect(res.status).toBe(200);
 				expect(normalizeDrive(await res.text())).toMatchInlineSnapshot(`
 			"Error: Oops!
-			    at Object.throw (file:///D:/virtual/esm/base64.cjs:9:14)
+			    at Object.throw (file:///D:/virtual/node/base64.cjs:9:14)
 			    at Object.fetch (file:///D:/virtual/esm/index.mjs:17:22)"
 			`);
 			} else {
@@ -285,12 +285,12 @@ describe("LocalRuntimeController", () => {
 			    at Object.fetch (file:///virtual/esm/index.mjs:15:19)"
 			`);
 
-				// Check stack traces from NodeJsCompatModule modules include file path
-				res = await fetch(new URL("/throw-nodejs-compat-module", url));
+				// Check stack traces from CommonJS modules include file path
+				res = await fetch(new URL("/throw-other-commonjs", url));
 				expect(res.status).toBe(200);
 				expect(await res.text()).toMatchInlineSnapshot(`
 			"Error: Oops!
-			    at Object.throw (file:///virtual/esm/base64.cjs:9:14)
+			    at Object.throw (file:///virtual/node/base64.cjs:9:14)
 			    at Object.fetch (file:///virtual/esm/index.mjs:17:22)"
 			`);
 			}

packages/wrangler/src/config/environment.ts

@@ -870,8 +870,7 @@ export type ConfigModuleRuleType =
 	| "Text"
 	| "Data"
 	| "PythonModule"
-	| "PythonRequirement"
-	| "NodeJsCompatModule";
+	| "PythonRequirement";
 
 export type TailConsumer = {
 	/** The name of the service tail events will be forwarded to. */

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

@@ -27,7 +27,6 @@ export const moduleTypeMimeType: {
 	text: "text/plain",
 	python: "text/x-python",
 	"python-requirement": "text/x-python-requirement",
-	"nodejs-compat-module": undefined,
 };
 
 function toMimeType(type: CfModuleType): string {