From efb35094f23ea48c51e8f0ec4f1aade47e91dd01 Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Sun, 12 May 2024 20:13:43 +0900 Subject: [PATCH] fix(esm): dont add namespace query to bare specifier (#21) --- src/cjs/api/module-resolve-filename.ts | 4 +-- src/esm/hook/resolve.ts | 22 ++++++------- src/utils/is-relative-path-pattern.ts | 1 - src/utils/path-utils.ts | 45 ++++++++++++++++++++++++++ tests/specs/api.ts | 10 +++--- 5 files changed, 64 insertions(+), 18 deletions(-) delete mode 100644 src/utils/is-relative-path-pattern.ts create mode 100644 src/utils/path-utils.ts diff --git a/src/cjs/api/module-resolve-filename.ts b/src/cjs/api/module-resolve-filename.ts index d9b1245a5..8e1a3061c 100644 --- a/src/cjs/api/module-resolve-filename.ts +++ b/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) diff --git a/src/esm/hook/resolve.ts b/src/esm/hook/resolve.ts index 890aabe1f..344272fe7 100644 --- a/src/esm/hook/resolve.ts +++ b/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) { diff --git a/src/utils/is-relative-path-pattern.ts b/src/utils/is-relative-path-pattern.ts deleted file mode 100644 index a7bc7003d..000000000 --- a/src/utils/is-relative-path-pattern.ts +++ /dev/null @@ -1 +0,0 @@ -export const isRelativePathPattern = /^\.{1,2}\//; diff --git a/src/utils/path-utils.ts b/src/utils/path-utils.ts new file mode 100644 index 000000000..3b3426b55 --- /dev/null +++ b/src/utils/path-utils.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' + ); +}; diff --git a/tests/specs/api.ts b/tests/specs/api.ts index cdf9b3975..ff4b0b88c 100644 --- a/tests/specs/api.ts +++ b/tests/specs/api.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({