fix: allow requireing alias modules (#6932)

Co-authored-by: Pete Bacon Darwin <[email protected]>

.changeset/nasty-melons-camp.md

@@ -0,0 +1,10 @@
+---
+"wrangler": minor
+---
+
+fix: allow `require`ing unenv aliased packages
+
+Before this PR `require`ing packages aliased in unenv would fail.
+That's because `require` would load the mjs file.
+
+This PR adds wraps the mjs file in a virtual ES module to allow `require`ing it.

fixtures/nodejs-hybrid-app/src/index.ts

@@ -26,18 +26,30 @@ export default {
 				return testPostgresLibrary(env, ctx);
 			case "/test-x509-certificate":
 				return testX509Certificate();
+			case "/test-require-alias":
+				return testRequireUenvAliasedPackages();
 		}
 
 		return new Response(
 			'<a href="query">Postgres query</a> | ' +
 				'<a href="test-process">Test process global</a> | ' +
 				'<a href="test-random">Test getRandomValues()</a> | ' +
-				'<a href="test-x509-certificate">Test X509Certificate</a>',
+				'<a href="test-x509-certificate">Test X509Certificate</a>' +
+				'<a href="test-require-alias">Test require unenv aliased packages</a>',
 			{ headers: { "Content-Type": "text/html; charset=utf-8" } }
 		);
 	},
 };
 
+function testRequireUenvAliasedPackages() {
+	const fetch = require("cross-fetch");
+	const supportsDefaultExports = typeof fetch === "function";
+	const supportsNamedExports = typeof fetch.Headers === "function";
+	return new Response(
+		supportsDefaultExports && supportsNamedExports ? `"OK!"` : `"KO!"`
+	);
+}
+
 function testX509Certificate() {
 	try {
 		new nodeCrypto.X509Certificate(`-----BEGIN CERTIFICATE-----

fixtures/nodejs-hybrid-app/tests/index.test.ts

@@ -63,4 +63,17 @@ describe("nodejs compat", () => {
 			await stop();
 		}
 	});
+
+	test("import unenv aliased packages", async ({ expect }) => {
+		const { ip, port, stop } = await runWranglerDev(
+			resolve(__dirname, "../src"),
+			["--port=0", "--inspector-port=0"]
+		);
+		try {
+			const response = await fetch(`http://${ip}:${port}/test-require-alias`);
+			await expect(response.text()).resolves.toBe(`"OK!"`);
+		} finally {
+			await stop();
+		}
+	});
 });

packages/wrangler/src/deployment-bundle/esbuild-plugins/hybrid-nodejs-compat.ts

@@ -6,6 +6,7 @@ import { getBasePath } from "../../paths";
 import type { Plugin, PluginBuild } from "esbuild";
 
 const REQUIRED_NODE_BUILT_IN_NAMESPACE = "node-built-in-modules";
+const REQUIRED_UNENV_ALIAS_NAMESPACE = "required-unenv-alias";
 
 export const nodejsHybridPlugin: () => Plugin = () => {
 	const { alias, inject, external } = env(nodeless, cloudflare);
@@ -14,7 +15,7 @@ export const nodejsHybridPlugin: () => Plugin = () => {
 		setup(build) {
 			errorOnServiceWorkerFormat(build);
 			handleRequireCallsToNodeJSBuiltins(build);
-			handleAliasedNodeJSPackages(build, alias, external);
+			handleUnenvAliasedPackages(build, alias, external);
 			handleNodeJSGlobals(build, inject);
 		},
 	};
@@ -55,7 +56,7 @@ function errorOnServiceWorkerFormat(build: PluginBuild) {
 }
 
 /**
- * We must convert `require()` calls for Node.js to a virtual ES Module that can be imported avoiding the require calls.
+ * We must convert `require()` calls for Node.js modules to a virtual ES Module that can be imported avoiding the require calls.
  * We do this by creating a special virtual ES module that re-exports the library in an onLoad handler.
  * The onLoad handler is triggered by matching the "namespace" added to the resolve.
  */
@@ -81,38 +82,76 @@ function handleRequireCallsToNodeJSBuiltins(build: PluginBuild) {
 	);
 }
 
-function handleAliasedNodeJSPackages(
+function handleUnenvAliasedPackages(
 	build: PluginBuild,
 	alias: Record<string, string>,
 	external: string[]
 ) {
 	// esbuild expects alias paths to be absolute
-	const aliasAbsolute = Object.fromEntries(
-		Object.entries(alias)
-			.map(([key, value]) => {
-				let resolvedAliasPath;
-				try {
-					resolvedAliasPath = require.resolve(value);
-				} catch (e) {
-					// this is an alias for package that is not installed in the current app => ignore
-					resolvedAliasPath = "";
-				}
+	const aliasAbsolute: Record<string, string> = {};
+	for (const [module, unresolvedAlias] of Object.entries(alias)) {
+		try {
+			aliasAbsolute[module] = require
+				.resolve(unresolvedAlias)
+				.replace(/\.cjs$/, ".mjs");
+		} catch (e) {
+			// this is an alias for package that is not installed in the current app => ignore
+		}
+	}
 
-				return [key, resolvedAliasPath.replace(/\.cjs$/, ".mjs")];
-			})
-			.filter((entry) => entry[1] !== "")
-	);
 	const UNENV_ALIAS_RE = new RegExp(
 		`^(${Object.keys(aliasAbsolute).join("|")})$`
 	);
 
 	build.onResolve({ filter: UNENV_ALIAS_RE }, (args) => {
+		const unresolvedAlias = alias[args.path];
+		// Convert `require()` calls for NPM packages to a virtual ES Module that can be imported avoiding the require calls.
+		// Note: Does not apply to Node.js packages that are handled in `handleRequireCallsToNodeJSBuiltins`
+		if (
+			args.kind === "require-call" &&
+			(unresolvedAlias.startsWith("unenv/runtime/npm/") ||
+				unresolvedAlias.startsWith("unenv/runtime/mock/"))
+		) {
+			return {
+				path: args.path,
+				namespace: REQUIRED_UNENV_ALIAS_NAMESPACE,
+			};
+		}
 		// Resolve the alias to its absolute path and potentially mark it as external
 		return {
 			path: aliasAbsolute[args.path],
-			external: external.includes(alias[args.path]),
+			external: external.includes(unresolvedAlias),
 		};
 	});
+
+	build.initialOptions.banner = { js: "", ...build.initialOptions.banner };
+	build.initialOptions.banner.js += dedent`
+		function __cf_cjs(esm) {
+		  const cjs = 'default' in esm ? esm.default : {};
+			for (const [k, v] of Object.entries(esm)) {
+				if (k !== 'default') {
+					Object.defineProperty(cjs, k, {
+						enumerable: true,
+						value: v,
+					});
+				}
+			}
+			return cjs;
+		}
+		`;
+
+	build.onLoad(
+		{ filter: /.*/, namespace: REQUIRED_UNENV_ALIAS_NAMESPACE },
+		({ path }) => {
+			return {
+				contents: dedent`
+        import * as esm from '${path}';
+				module.exports = __cf_cjs(esm);
+				`,
+				loader: "js",
+			};
+		}
+	);
 }
 
 /**