fix: createRequire leaking into CommonJS (#3)

fixes #103

Author: privatenumber

package.json

@@ -23,7 +23,8 @@
 	"files": [
 		"dist"
 	],
-	"bin": "./dist/cli.js",
+	"type": "module",
+	"bin": "./dist/cli.mjs",
 	"imports": {
 		"typescript": "./src/local-typescript-loader.ts"
 	},
@@ -57,13 +58,15 @@
 		"@rollup/pluginutils": "^5.1.4",
 		"esbuild": "^0.24.2",
 		"magic-string": "^0.30.17",
-		"rollup": "^4.29.1"
+		"rollup": "^4.29.1",
+		"rollup-pluginutils": "^2.8.2"
 	},
 	"devDependencies": {
 		"@types/node": "^22.10.2",
 		"@types/react": "^18.3.5",
 		"clean-pkg-json": "^1.2.0",
 		"cleye": "^1.3.2",
+		"estree-walker": "^3.0.3",
 		"execa": "9.3.0",
 		"fs-fixture": "^2.6.0",
 		"get-node": "^15.0.1",

pnpm-lock.yaml

@@ -11,13 +11,13 @@ import dynamicImportVars from '@rollup/plugin-dynamic-import-vars';
 import type { PackageJson } from 'type-fest';
 import type { TsConfigResult } from 'get-tsconfig';
 import type { ExportEntry, AliasMap } from '../types.js';
-import { isFormatEsm, createRequire } from './rollup-plugins/create-require.js';
 import { esbuildTransform, esbuildMinify } from './rollup-plugins/esbuild.js';
 import { externalizeNodeBuiltins } from './rollup-plugins/externalize-node-builtins.js';
 import { patchBinary } from './rollup-plugins/patch-binary.js';
 import { resolveTypescriptMjsCts } from './rollup-plugins/resolve-typescript-mjs-cjs.js';
 import { resolveTsconfigPaths } from './rollup-plugins/resolve-tsconfig-paths.js';
 import { stripHashbang } from './rollup-plugins/strip-hashbang.js';
+import { esmInjectCreateRequire } from './rollup-plugins/esm-inject-create-require.js';
 import { getExternalDependencies } from './parse-package-json/get-external-dependencies.js';
 
 type Options = {
@@ -134,10 +134,13 @@ const getConfig = {
 						: []
 				),
 				stripHashbang(),
-				commonjs(),
 				json(),
 				esbuildTransform(esbuildConfig),
-				createRequire(),
+				commonjs({
+					ignoreDynamicRequires: true,
+					extensions: ['.js', '.ts', '.jsx', '.tsx'],
+					transformMixedEsModules: true,
+				}),
 				dynamicImportVars({
 					warnOnError: true,
 				}),
@@ -147,6 +150,7 @@ const getConfig = {
 						: []
 				),
 				patchBinary(executablePaths),
+				esmInjectCreateRequire(),
 			],
 			output: [] as unknown as Output,
 			external: [] as (string | RegExp)[],
@@ -253,9 +257,6 @@ export const getRollupConfigs = async (
 				format: exportEntry.type,
 				chunkFileNames: `[name]-[hash]${extension}`,
 				sourcemap: flags.sourcemap,
-				plugins: [
-					isFormatEsm(exportEntry.type === 'module'),
-				],
 
 				/**
 				 * Preserve source path in dist path

src/utils/get-rollup-configs.ts

@@ -0,0 +1,60 @@
+import MagicString from 'magic-string';
+import { attachScopes, type AttachedScope } from 'rollup-pluginutils';
+import { walk } from 'estree-walker';
+import type { Plugin } from 'rollup';
+
+export const esmInjectCreateRequire = (): Plugin => {
+	const createRequire = 'import{createRequire as _pkgrollCR}from"node:module";const require=_pkgrollCR(import.meta.url);';
+
+	return {
+		name: 'esmInjectCreateRequire',
+		renderChunk(code, _chunk, options) {
+			if (
+				options.format !== 'es'
+				|| !/\brequire\b/.test(code)
+			) {
+				return null;
+			}
+
+			const ast = this.parse(code);
+			let currentScope = attachScopes(ast, 'scope');
+			let injectionNeeded = false;
+
+			walk(ast, {
+				enter(node) {
+					// Not all nodes have scopes
+					if (node.scope) {
+						currentScope = node.scope as AttachedScope;
+					}
+					if (
+						node.type === 'Identifier'
+						&& node.name === 'require'
+						// If the current scope (or its parents) does not contain 'require'
+						&& !currentScope.contains('require')
+					) {
+						injectionNeeded = true;
+
+						// No need to continue if one instance is found
+						this.skip();
+					}
+				},
+				leave: (node) => {
+					if (node.scope) {
+						currentScope = currentScope.parent!;
+					}
+				},
+			});
+
+			if (!injectionNeeded) {
+				return null;
+			}
+
+			const magicString = new MagicString(code);
+			magicString.prepend(createRequire);
+			return {
+				code: magicString.toString(),
+				map: magicString.generateMap({ hires: true }),
+			};
+		},
+	};
+};

src/utils/rollup-plugins/create-require.ts

@@ -1,3 +1,4 @@
+import { pathToFileURL } from 'node:url';
 import { testSuite, expect } from 'manten';
 import { createFixture } from 'fs-fixture';
 import { pkgroll } from '../../utils.js';
@@ -30,10 +31,8 @@ export default testSuite(({ describe }, nodePath: string) => {
 			expect(content).not.toMatch('exports.foo=foo');
 
 			// Minification should preserve name
-			expect(
-				// eslint-disable-next-line @typescript-eslint/no-require-imports
-				require(fixture.getPath('dist/target.js')).functionName,
-			).toBe('preservesName');
+			const { functionName } = await import(pathToFileURL(fixture.getPath('dist/target.js')).toString());
+			expect(functionName).toBe('preservesName');
 		});
 	});
 });

src/utils/rollup-plugins/esm-inject-create-require.ts

@@ -1,4 +1,5 @@
 import fs from 'node:fs/promises';
+import { pathToFileURL } from 'node:url';
 import { testSuite, expect } from 'manten';
 import { createFixture } from 'fs-fixture';
 import { pkgroll } from '../../utils.js';
@@ -181,7 +182,7 @@ export default testSuite(({ describe }, nodePath: string) => {
 			expect(content).toMatch('export { sayHello }');
 		});
 
-		test('require() works in esm', async () => {
+		test('require() gets converted to import in esm', async () => {
 			await using fixture = await createFixture({
 				...packageFixture(),
 				'package.json': createPackageJson({
@@ -202,7 +203,8 @@ export default testSuite(({ describe }, nodePath: string) => {
 			expect(js).not.toMatch('createRequire');
 
 			const mjs = await fixture.readFile('dist/require.mjs', 'utf8');
-			expect(mjs).toMatch('createRequire');
+			expect(mjs).not.toMatch('require(');
+			expect(mjs).toMatch(/import . from"fs"/);
 		});
 
 		test('conditional require() no side-effects', async () => {
@@ -243,9 +245,55 @@ export default testSuite(({ describe }, nodePath: string) => {
 
 			const content = await fixture.readFile('dist/conditional-require.mjs', 'utf8');
 			expect(content).not.toMatch('\tconsole.log(\'side effect\');');
+			expect(content).not.toMatch('require(');
+			expect(content).toMatch('"development"');
+		});
+
+		describe('injects createRequire', ({ test }) => {
+			test('dynamic require should get a createRequire', async () => {
+				await using fixture = await createFixture({
+					'src/dynamic-require.ts': 'require((() => \'fs\')());',
+					'package.json': createPackageJson({
+						main: './dist/dynamic-require.mjs',
+					}),
+				});
+
+				const pkgrollProcess = await pkgroll([], {
+					cwd: fixture.path,
+					nodePath,
+				});
+
+				expect(pkgrollProcess.exitCode).toBe(0);
+				expect(pkgrollProcess.stderr).toBe('');
+
+				const content = await fixture.readFile('dist/dynamic-require.mjs', 'utf8');
+				expect(content).toMatch('createRequire');
+				expect(content).toMatch('(import.meta.url)');
+
+				// Shouldn't throw
+				await import(pathToFileURL(fixture.getPath('dist/dynamic-require.mjs')).toString());
+			});
 
-			const [, createRequireMangledVariable] = content.toString().match(/createRequire as (\w+)/)!;
-			expect(content).not.toMatch(`${createRequireMangledVariable}(`);
+			test('defined require should not get a createRequire', async () => {
+				await using fixture = await createFixture({
+					'src/dynamic-require.ts': 'const require = ()=>{}; require((() => \'fs\')());',
+					'package.json': createPackageJson({
+						main: './dist/dynamic-require.mjs',
+					}),
+				});
+
+				const pkgrollProcess = await pkgroll([], {
+					cwd: fixture.path,
+					nodePath,
+				});
+
+				expect(pkgrollProcess.exitCode).toBe(0);
+				expect(pkgrollProcess.stderr).toBe('');
+
+				const content = await fixture.readFile('dist/dynamic-require.mjs', 'utf8');
+				expect(content).not.toMatch('createRequire');
+				expect(content).not.toMatch('(import.meta.url)');
+			});
 		});
 
 		test('dynamic imports', async () => {

tests/specs/builds/minification.ts

@@ -1,7 +1,7 @@
 import path from 'node:path';
 import { execa, type Options } from 'execa';
 
-const pkgrollBinPath = path.resolve('./dist/cli.js');
+const pkgrollBinPath = path.resolve('./dist/cli.mjs');
 
 export const pkgroll = async (
 	cliArguments: string[],