fix(esm): dont add namespace query to bare specifier (#21)

Author: privatenumber

src/cjs/api/module-resolve-filename.ts

@@ -3,7 +3,7 @@ import Module from 'node:module';
 import { createPathsMatcher } from 'get-tsconfig';
 import { resolveTsPath } from '../../utils/resolve-ts-path.js';
 import type { NodeError } from '../../types.js';
-import { isRelativePathPattern } from '../../utils/is-relative-path-pattern.js';
+import { isRelativePath } from '../../utils/path-utils.js';
 import {
 	isTsFilePatten,
 	tsconfig,
@@ -73,7 +73,7 @@ export const resolveFilename: ResolveFilename = (
 		tsconfigPathsMatcher
 
 		// bare specifier
-		&& !isRelativePathPattern.test(request)
+		&& !isRelativePath(request)
 
 		// Dependency paths should not be resolved using tsconfig.json
 		&& !parent?.filename?.includes(nodeModulesPath)

src/esm/hook/resolve.ts

@@ -5,7 +5,7 @@ import type {
 } from 'node:module';
 import { resolveTsPath } from '../../utils/resolve-ts-path.js';
 import type { NodeError } from '../../types.js';
-import { isRelativePathPattern } from '../../utils/is-relative-path-pattern.js';
+import { requestAcceptsQuery } from '../../utils/path-utils.js';
 import {
 	tsconfigPathsMatcher,
 	tsExtensionsPattern,
@@ -121,14 +121,8 @@ export const resolve: resolve = async (
 		return nextResolve(specifier, context);
 	}
 
-	const isPath = (
-		specifier.startsWith(fileProtocol)
-		|| path.isAbsolute(specifier)
-		|| isRelativePathPattern.test(specifier)
-	);
-
 	const parentNamespace = context.parentURL && getNamespace(context.parentURL);
-	if (isPath) {
+	if (requestAcceptsQuery(specifier)) {
 		// Inherit namespace from parent
 		let requestNamespace = getNamespace(specifier);
 		if (parentNamespace && !requestNamespace) {
@@ -190,9 +184,15 @@ export const resolve: resolve = async (
 
 	try {
 		const resolved = await resolveExplicitPath(nextResolve, specifier, context);
-		const resolvedNamespace = getNamespace(resolved.url);
-		if (parentNamespace && !resolvedNamespace) {
-			resolved.url += `${resolved.url.includes('?') ? '&' : '?'}${namespaceQuery}${parentNamespace}`;
+		// Could be a core Node module (e.g. `fs`)
+		if (requestAcceptsQuery(resolved.url)) {
+			const resolvedNamespace = getNamespace(resolved.url);
+			if (
+				parentNamespace
+				&& !resolvedNamespace
+			) {
+				resolved.url += `${resolved.url.includes('?') ? '&' : '?'}${namespaceQuery}${parentNamespace}`;
+			}
 		}
 		return resolved;
 	} catch (error) {

src/utils/is-relative-path-pattern.ts

@@ -0,0 +1,45 @@
+import path from 'node:path';
+
+/**
+ * Prior to calling this function, it's expected that Windows paths have been filtered out
+ * via path.isAbsolute()
+ *
+ * Windows paths cannot be correctly parsed (e.g. new URL('C:\Users\Example\file.txt')
+ */
+const getScheme = (url: string) => {
+	const schemeIndex = url.indexOf(':');
+	if (schemeIndex === -1) { return; }
+	return url.slice(0, schemeIndex);
+};
+
+export const isRelativePath = (request: string) => (
+	request[0] === '.'
+	&& (
+		request[1] === '/'
+		|| (request[1] === '.' || request[2] === '/')
+	)
+);
+
+const isUnixPath = (request: string) => (
+	isRelativePath(request)
+	|| path.isAbsolute(request)
+);
+
+// In Node, bare specifiers (packages and core modules) do not accept queries
+export const requestAcceptsQuery = (request: string) => {
+	// ./foo.js?query
+	// /foo.js?query in UNIX
+	if (isUnixPath(request)) {
+		return true;
+	}
+
+	const scheme = getScheme(request);
+	return (
+		// Expected to be file, https, etc...
+		scheme
+
+		// node:url maps to a bare-specifier, which does not accept queries
+		// But URLs like file:// or https:// do
+		&& scheme !== 'node'
+	);
+};

src/utils/path-utils.ts

@@ -26,7 +26,7 @@ const tsFiles = {
 			type: 'module',
 			exports: './index.js',
 		}),
-		'index.js': 'export const bar = "bar"',
+		'index.js': 'import "node:process"; export const bar = "bar";',
 	},
 };
 
@@ -254,13 +254,14 @@ export default testSuite(({ describe }, node: NodeApis) => {
 							nodePath: node.path,
 							nodeOptions: [],
 						});
-						expect(stdout).toBe('file.ts\nfoo.ts\nbar.ts\nindex.js');
+						expect(stdout).toBe('file.ts\nfoo.ts\nbar.ts\nindex.js\nnode:process');
 					});
 
 					test('namespace & onImport', async () => {
 						await using fixture = await createFixture({
 							'package.json': JSON.stringify({ type: 'module' }),
 							'register.mjs': `
+							import { setTimeout } from 'node:timers/promises';
 							import { register } from ${JSON.stringify(tsxEsmApiPath)};
 
 							const api = register({
@@ -272,7 +273,7 @@ export default testSuite(({ describe }, node: NodeApis) => {
 
 							await api.import('./file', import.meta.url);
 
-							api.unregister();
+							await setTimeout(100)
 							`,
 							...tsFiles,
 						});
@@ -394,6 +395,7 @@ export default testSuite(({ describe }, node: NodeApis) => {
 						await using fixture = await createFixture({
 							'package.json': JSON.stringify({ type: 'module' }),
 							'import.mjs': `
+							import { setTimeout } from 'node:timers/promises';
 							import { tsImport } from ${JSON.stringify(tsxEsmApiPath)};
 							const dependenciesA = [];
 							await tsImport('./file.ts', {
@@ -412,7 +414,7 @@ export default testSuite(({ describe }, node: NodeApis) => {
 							});
 
 							// wait for async import() to finish
-							await new Promise((resolve) => setTimeout(resolve, 10));
+							await setTimeout(100)
 
 							if (JSON.stringify(dependenciesA) !== JSON.stringify(dependenciesB)) {
 								console.log({