fix(esm): ignore transforming .js files with CJS syntax (#40)

fixes #577

src/cjs/api/module-extensions.ts

@@ -3,7 +3,7 @@ import Module from 'node:module';
 import type { TransformOptions } from 'esbuild';
 import { transformSync } from '../../utils/transform/index.js';
 import { transformDynamicImport } from '../../utils/transform/transform-dynamic-import.js';
-import { isESM } from '../../utils/esm-pattern.js';
+import { isESM } from '../../utils/es-module-lexer.js';
 import { shouldApplySourceMap, inlineSourceMap } from '../../source-map.js';
 import { parent } from '../../utils/ipc/client.js';
 import { fileMatcher } from '../../utils/tsconfig.js';

src/esm/hook/load.ts

@@ -10,6 +10,7 @@ import { parent } from '../../utils/ipc/client.js';
 import type { Message } from '../types.js';
 import { fileMatcher } from '../../utils/tsconfig.js';
 import { isJsonPattern, tsExtensionsPattern } from '../../utils/path-utils.js';
+import { isESM } from '../../utils/es-module-lexer.js';
 import { getNamespace } from './utils.js';
 import { data } from './initialize.js';
 
@@ -70,34 +71,41 @@ export const load: LoadHook = async (
 		&& loaded.responseURL?.startsWith('file:') // Could be data:
 		&& !filePath.endsWith('.cjs') // CJS syntax doesn't need to be transformed for interop
 	) {
-		/**
-		 * es or cjs module lexer unfortunately cannot be used because it doesn't support
-		 * typescript syntax
-		 *
-		 * While the full code is transformed, only the exports are used for parsing.
-		 * In fact, the code can't even run because imports cannot be resolved relative
-		 * from the data: URL.
-		 *
-		 * TODO: extract exports only
-		 */
-		const transformed = await transform(
-			await readFile(new URL(url), 'utf8'),
-			filePath,
-			{
-				format: 'cjs',
-
-				// CJS Annotations for Node
-				platform: 'node',
-				// TODO: disable source maps
-			},
-		);
-
-		const parameters = new URLSearchParams({ filePath });
-		if (urlNamespace) {
-			parameters.set('namespace', urlNamespace);
+		const code = await readFile(new URL(url), 'utf8');
+
+		// if the file extension is .js, only transform if using esm syntax
+		if (!filePath.endsWith('.js') || isESM(code)) {
+			/**
+			 * es or cjs module lexer unfortunately cannot be used because it doesn't support
+			 * typescript syntax
+			 *
+			 * While the full code is transformed, only the exports are used for parsing.
+			 * In fact, the code can't even run because imports cannot be resolved relative
+			 * from the data: URL.
+			 *
+			 * TODO: extract exports only
+			 */
+			const transformed = await transform(
+				code,
+				filePath,
+				{
+					format: 'cjs',
+
+					// CJS Annotations for Node
+					platform: 'node',
+					// TODO: disable source maps
+				},
+			);
+
+			const parameters = new URLSearchParams({ filePath });
+			if (urlNamespace) {
+				parameters.set('namespace', urlNamespace);
+			}
+
+			// TODO: re-exports from relative paths cant get detected because of the data URL
+			loaded.responseURL = `data:text/javascript,${encodeURIComponent(transformed.code)}?${parameters.toString()}`;
+			return loaded;
 		}
-		loaded.responseURL = `data:text/javascript,${encodeURIComponent(transformed.code)}?${parameters.toString()}`;
-		return loaded;
 	}
 
 	// CommonJS and Internal modules (e.g. node:*)

src/utils/es-module-lexer.ts

@@ -16,3 +16,39 @@ export const parseEsm = (
 		? parseWasm(code, filename)
 		: parseJs(code, filename)
 );
+
+/*
+Previously, this regex was used as a naive ESM catch,
+but turns out regex is slower than the lexer so removing
+it made the check faster.
+
+Catches:
+import a from 'b'
+import 'b';
+import('b');
+export{a};
+export default a;
+
+Doesn't catch:
+EXPORT{a}
+exports.a = 1
+module.exports = 1
+
+const esmPattern = /\b(?:import|export)\b/;
+*/
+
+export const isESM = (code: string) => {
+	if (!code.includes('import') && !code.includes('export')) {
+		return false;
+	}
+	try {
+		const hasModuleSyntax = parseEsm(code)[3];
+		return hasModuleSyntax;
+	} catch {
+		/**
+		 * If it fails to parse, there's a syntax error
+		 * Let esbuild handle it for better error messages
+		 */
+		return true;
+	}
+};

tests/fixtures.ts

@@ -257,6 +257,16 @@ export const files = {
 			}),
 			'index.js': syntaxLowering,
 			'ts.ts': syntaxLowering,
+			'cjs.js': `
+			const _ = exports;
+			const cjsJs = true;
+			_.cjsJs = cjsJs;
+
+			// Annotate CommonJS exports for Node
+			0 && (module.exports = {
+				cjsJs,
+			});
+			`,
 		},
 		'pkg-module': {
 			'package.json': createPackageJson({

tests/specs/smoke.ts

@@ -33,10 +33,14 @@ export default testSuite(async ({ describe }, { tsx, supports }: NodeApis) => {
 						import 'node:fs';
 
 						import * as pkgCommonjs from 'pkg-commonjs';
+
+						// Named exports from CommonJS
+						import { cjsJs } from 'pkg-commonjs/cjs.js';
+
 						import * as pkgModule from 'pkg-module';
 						import 'pkg-module/empty-export'; // implicit directory & extension
 
-						// .js
+						// .js in esm syntax
 						import * as js from './js/index.js';
 						import './js/index.js?query=123';
 						import './js/index';
@@ -190,7 +194,10 @@ export default testSuite(async ({ describe }, { tsx, supports }: NodeApis) => {
 						import 'pkg-commonjs/ts.js';
 						import 'pkg-module/ts.js';
 
-						// .js
+						// Named exports from CommonJS
+						import { cjsJs } from 'pkg-commonjs/cjs.js';
+
+						// .js in esm syntax
 						import * as js from './js/index.js';
 						import './js/index.js?query=123';
 						import './js/index';
@@ -354,6 +361,7 @@ export default testSuite(async ({ describe }, { tsx, supports }: NodeApis) => {
 							NODE_V8_COVERAGE: 'coverage',
 						},
 					});
+
 					onTestFail((error) => {
 						console.error(error);
 						console.log(p);