fix: support ambiguous package (#79)

Author: privatenumber

src/esm/hook/load.ts

@@ -195,82 +195,86 @@ let load: LoadHook = async (
 		loaded,
 	});
 
-	if (
-		isFeatureSupported(loadReadFromSource)
-		&& loaded.format === 'commonjs'
-		&& filePath.endsWith('.cjs')
-	) {
-		let code = await readFile(new URL(url), 'utf8');
-		// Contains native ESM check
-		const transformed = transformDynamicImport(filePath, code);
-		if (transformed) {
-			code = inlineSourceMap(transformed);
+	if (loaded.format === 'commonjs') {
+		if (
+			isFeatureSupported(loadReadFromSource)
+			&& filePath.endsWith('.cjs')
+		) {
+			let code = await readFile(new URL(url), 'utf8');
+			// Contains native ESM check
+			const transformed = transformDynamicImport(filePath, code);
+			if (transformed) {
+				code = inlineSourceMap(transformed);
+			}
+			loaded.source = code;
+			loaded.shortCircuit = true;
+			return loaded;
 		}
-		loaded.source = code;
-		loaded.shortCircuit = true;
-		return loaded;
-	}
-
-	if (
-		loaded.format === 'commonjs'
-		&& isFeatureSupported(esmLoadReadFile)
-		&& loaded.responseURL?.startsWith('file:') // Could be data:
-		&& !filePath.endsWith('.cjs') // CJS syntax doesn't need to be transformed for interop
-	) {
-		const code = await readFile(new URL(url), 'utf8');
 
-		// if the file extension is .js, only transform if using esm syntax
 		if (
-			// TypeScript files
-			!filePath.endsWith('.js')
-
-			// ESM syntax in CommonJS type package
-			|| isESM(code)
+			isFeatureSupported(esmLoadReadFile)
+			&& 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.
-			 *
-			 * This should pre-compile for the CJS loader to have a cache hit
-			 *
-			 * I considered extracting the CJS exports from esbuild via (0&&(module.exports={})
-			 * to minimize the data URL size but this only works for ESM->CJS and not CTS files
-			 * which are already in CJS syntax.
-			 * In CTS, module.exports can be written in any pattern.
-			 */
-			const transformed = transformSync(
-				code,
-				url,
-				{
-					tsconfigRaw: fileMatcher?.(filePath) as TransformOptions['tsconfigRaw'],
-				},
-			);
+			const code = await readFile(new URL(url), 'utf8');
 
-			if (isFeatureSupported(loadReadFromSource)) {
-				/**
-				 * Compile ESM to CJS
-				 * In v22.15, the CJS loader logic is now moved to the ESM loader
-				 */
-				loaded.source = inlineSourceMap(transformed);
-			} else {
+			// if the file extension is .js, only transform if using esm syntax
+			if (
+				// TypeScript files
+				!filePath.endsWith('.js')
+
+				// ESM syntax in CommonJS type package
+				|| isESM(code)
+			) {
 				/**
-				 * This tricks Node into thinking the file is a data URL so it doesn't try to read from disk
-				 * to parse the CJS exports
+				 * 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.
+				 *
+				 * This should pre-compile for the CJS loader to have a cache hit
+				 *
+				 * I considered extracting the CJS exports from esbuild via (0&&(module.exports={})
+				 * to minimize the data URL size but this only works for ESM->CJS and not CTS files
+				 * which are already in CJS syntax.
+				 * In CTS, module.exports can be written in any pattern.
 				 */
-				const filePathWithNamespace = urlNamespace ? `${filePath}?namespace=${encodeURIComponent(urlNamespace)}` : filePath;
-				loaded.responseURL = `data:text/javascript,${encodeURIComponent(transformed.code)}?filePath=${encodeURIComponent(filePathWithNamespace)}`;
+				const transformed = transformSync(
+					code,
+					url,
+					{
+						tsconfigRaw: fileMatcher?.(filePath) as TransformOptions['tsconfigRaw'],
+					},
+				);
+
+				if (isFeatureSupported(loadReadFromSource)) {
+					/**
+					 * Compile ESM to CJS
+					 * In v22.15, the CJS loader logic is now moved to the ESM loader
+					 */
+					loaded.source = inlineSourceMap(transformed);
+				} else {
+					/**
+					 * This tricks Node into thinking the file is a data URL so it doesn't try to
+					 * read from disk to parse the CJS exports
+					 */
+					const filePathWithNamespace = urlNamespace ? `${filePath}?namespace=${encodeURIComponent(urlNamespace)}` : filePath;
+					loaded.responseURL = `data:text/javascript,${encodeURIComponent(transformed.code)}?filePath=${encodeURIComponent(filePathWithNamespace)}`;
+				}
+
+				return loaded;
 			}
 
-			log(3, 'returning CJS export annotation', loaded);
-			return loaded;
+			if (!loaded.source) {
+				loaded.source = code;
+				return loaded;
+			}
 		}
 	}
 
-	// CommonJS and Internal modules (e.g. node:*)
+	// Internal modules (e.g. node:*)
 	if (!loaded.source) {
 		return loaded;
 	}

src/esm/hook/resolve.ts

@@ -348,6 +348,10 @@ let resolve: ResolveHook = async (
 		nextResolve,
 	);
 
+	log(2, 'nextResolve', {
+		resolved,
+	});
+
 	if (resolved.format === 'builtin') {
 		return resolved;
 	}
@@ -359,6 +363,10 @@ let resolve: ResolveHook = async (
 		&& resolved.url.startsWith(fileUrlPrefix)
 	) {
 		resolved.format = await getFormatFromFileUrl(resolved.url);
+		log(2, 'getFormatFromFileUrl', {
+			resolved,
+			format: resolved.format,
+		});
 	}
 
 	if (query) {

src/esm/hook/utils.ts

@@ -1,6 +1,6 @@
 import path from 'node:path';
 import type { ModuleFormat } from 'node:module';
-import { tsExtensionsPattern } from '../../utils/path-utils.js';
+import { tsExtensions } from '../../utils/path-utils.js';
 import { getPackageType } from './package-json.js';
 
 const getFormatFromExtension = (fileUrl: string): ModuleFormat | undefined => {
@@ -24,7 +24,9 @@ export const getFormatFromFileUrl = (fileUrl: string) => {
 	}
 
 	// ts, tsx, jsx
-	if (tsExtensionsPattern.test(fileUrl)) {
+	const { pathname } = new URL(fileUrl);
+	const extension = path.extname(pathname);
+	if (extension === '.js' || tsExtensions.includes(extension)) {
 		return getPackageType(fileUrl);
 	}
 };

src/utils/path-utils.ts

@@ -46,6 +46,8 @@ export const requestAcceptsQuery = (request: string) => {
 
 export const fileUrlPrefix = 'file://';
 
+export const tsExtensions = ['.ts', '.tsx', '.jsx', '.mts', '.cts'];
+
 export const tsExtensionsPattern = /\.([cm]?ts|[tj]sx)($|\?)/;
 
 export const cjsExtensionPattern = /[/\\].+\.(?:cts|cjs)(?:$|\?)/;

tests/fixtures.ts

@@ -294,27 +294,6 @@ export const files = {
 			});
 			`,
 		},
-		// TODO: Package with no type field but ESM syntax
-		// (also check this in the app itself, not just in node_modules)
-		'pkg-ambiguous': {
-			'package.json': createPackageJson({
-				type: 'commonjs',
-				main: './index.js',
-			}),
-			'index.ts': 'throw new Error("should prefer .js over .ts in node_modules")',
-			'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({
 				type: 'module',

tests/specs/smoke.ts

@@ -15,12 +15,12 @@ const wasmPathUrl = pathToFileURL(wasmPath).toString();
 export default testSuite(async ({ describe }, { tsx, supports, version }: NodeApis) => {
 	describe('Smoke', ({ describe, test }) => {
 		for (const packageType of packageTypes) {
-			const isCommonJs = packageType === 'commonjs';
+			const isCommonJs = packageType !== 'module';
 
-			describe(packageType, ({ test }) => {
+			describe(`package type: ${packageType ?? 'undefined'}`, ({ test }) => {
 				test('from .js', async ({ onTestFail }) => {
 					await using fixture = await createFixture({
-						'package.json': createPackageJson({ type: packageType }),
+						'package.json': createPackageJson(packageType ? { type: packageType } : {}),
 						'import-from-js.js': outdent`
 						import assert from 'assert';
 						import { expectErrors } from 'expect-errors';
@@ -200,7 +200,7 @@ export default testSuite(async ({ describe }, { tsx, supports, version }: NodeAp
 
 				test('from .ts', async ({ onTestFail }) => {
 					await using fixture = await createFixture({
-						'package.json': createPackageJson({ type: packageType }),
+						'package.json': createPackageJson(packageType ? { type: packageType } : {}),
 
 						'import-from-ts.ts': ({ fixturePath }) => outdent`
 						import assert from 'assert';
@@ -464,7 +464,7 @@ export default testSuite(async ({ describe }, { tsx, supports, version }: NodeAp
 
 				test('resolve ts in exports', async () => {
 					await using fixture = await createFixture({
-						'package.json': createPackageJson({ type: packageType }),
+						'package.json': createPackageJson(packageType ? { type: packageType } : {}),
 						'index.ts': `
 						import A from 'pkg'
 						console.log(A satisfies 2)
@@ -493,7 +493,7 @@ export default testSuite(async ({ describe }, { tsx, supports, version }: NodeAp
 				if (!version.startsWith('18.')) {
 					test('resolve ts in main', async ({ onTestFail }) => {
 						await using fixture = await createFixture({
-							'package.json': createPackageJson({ type: packageType }),
+							'package.json': createPackageJson(packageType ? { type: packageType } : {}),
 							'index.ts': `
 							import A from 'pkg'
 							console.log(A satisfies 2);

tests/specs/tsconfig.ts

@@ -9,11 +9,11 @@ import { packageTypes } from '../utils/package-types.js';
 export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
 	describe('tsconfig', ({ describe }) => {
 		for (const packageType of packageTypes) {
-			describe(packageType, async ({ describe, onFinish }) => {
+			describe(`package type: ${packageType ?? 'undefined'}`, async ({ describe, onFinish }) => {
 				const fixture = await createFixture({
 					...expectErrors,
 
-					'package.json': createPackageJson({ type: packageType }),
+					'package.json': createPackageJson(packageType ? { type: packageType } : {}),
 
 					'tsconfig.json': createTsconfig({
 						compilerOptions: {
@@ -115,7 +115,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
 				describe('detected tsconfig', ({ test }) => {
 					test('invalid tsconfig should be ignored', async () => {
 						await using fixture = await createFixture({
-							'package.json': createPackageJson({ type: packageType }),
+							'package.json': createPackageJson(packageType ? { type: packageType } : {}),
 							'tsconfig.json': createTsconfig({
 								extends: 'doesnt-exist',
 							}),
@@ -141,7 +141,7 @@ export default testSuite(async ({ describe }, { tsx }: NodeApis) => {
 				describe('custom tsconfig', ({ test }) => {
 					test('invalid tsconfig should error', async () => {
 						await using fixture = await createFixture({
-							'package.json': createPackageJson({ type: packageType }),
+							'package.json': createPackageJson(packageType ? { type: packageType } : {}),
 							'tsconfig.json': createTsconfig({
 								extends: 'doesnt-exist',
 							}),

tests/utils/package-types.ts

@@ -1,5 +1,5 @@
 export const packageTypes = [
-	// undefined, // TODO: Add ambiguous package type
+	undefined,
 	'module',
 	'commonjs',
 ] as const;