Skip to content

Commit

Permalink
fix(esm): dont add namespace query to bare specifier (#21)
Browse files Browse the repository at this point in the history
  • Loading branch information
privatenumber authored May 12, 2024
1 parent b273848 commit efb3509
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 18 deletions.
4 changes: 2 additions & 2 deletions src/cjs/api/module-resolve-filename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
22 changes: 11 additions & 11 deletions src/esm/hook/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion src/utils/is-relative-path-pattern.ts

This file was deleted.

45 changes: 45 additions & 0 deletions src/utils/path-utils.ts
Original file line number Diff line number Diff line change
@@ -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'
);
};
10 changes: 6 additions & 4 deletions tests/specs/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";',
},
};

Expand Down Expand Up @@ -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({
Expand All @@ -272,7 +273,7 @@ export default testSuite(({ describe }, node: NodeApis) => {
await api.import('./file', import.meta.url);
api.unregister();
await setTimeout(100)
`,
...tsFiles,
});
Expand Down Expand Up @@ -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', {
Expand All @@ -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({
Expand Down

0 comments on commit efb3509

Please sign in to comment.