Skip to content

Commit

Permalink
fix(cjs): leaking implicit extension resolver
Browse files Browse the repository at this point in the history
  • Loading branch information
privatenumber committed Jun 28, 2024
1 parent b94482d commit ae2a1bc
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 125 deletions.
100 changes: 49 additions & 51 deletions src/cjs/api/module-resolve-filename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,71 +156,69 @@ const resolveRequest = (
export const createResolveFilename = (
nextResolve: ResolveFilename,
namespace?: string,
): ResolveFilename => {
): ResolveFilename => (
request,
parent,
isMain,
options,
) => {
const resolve: SimpleResolve = request_ => nextResolve(
request_,
parent,
isMain,
options,
);

request = interopCjsExports(request);

if (parent?.filename) {
const filePath = getOriginalFilePath(parent.filename);
if (filePath) {
parent.filename = filePath.split('?')[0];
}
}

// Strip query string
const requestAndQuery = request.split('?');
const searchParams = new URLSearchParams(requestAndQuery[1]);

// Inherit parent namespace if it exists
if (parent?.filename) {
const parentQuery = new URLSearchParams(parent.filename.split('?')[1]);
const parentNamespace = parentQuery.get('namespace');
if (parentNamespace) {
searchParams.append('namespace', parentNamespace);
}
}

// If request namespace doesnt match the namespace, ignore
if ((searchParams.get('namespace') ?? undefined) !== namespace) {
return resolve(request);
}

if (namespace) {
/**
* When namespaced, the loaders are registered to the extensions in a hidden way
* so Node's built-in resolver will not try those extensions
*
* To support implicit extensions, we need to wrap the resolver with our own
* To support implicit extensions, we need to enhance the resolver with our own
* re-implementation of the implicit extension resolution
*/
nextResolve = createImplicitResolver(nextResolve);
}

return (
request,
parent,
isMain,
options,
) => {
const resolve: SimpleResolve = request_ => nextResolve(
request_,
parent,
isMain,
options,
);

request = interopCjsExports(request);

if (parent?.filename) {
const filePath = getOriginalFilePath(parent.filename);
if (filePath) {
parent.filename = filePath.split('?')[0];
}
}

// Strip query string
const requestAndQuery = request.split('?');
const searchParams = new URLSearchParams(requestAndQuery[1]);
let resolved = resolveRequest(requestAndQuery[0], parent, resolve);

// Inherit parent namespace if it exists
if (parent?.filename) {
const parentQuery = new URLSearchParams(parent.filename.split('?')[1]);
const parentNamespace = parentQuery.get('namespace');
if (parentNamespace) {
searchParams.append('namespace', parentNamespace);
}
}

// If request namespace doesnt match the namespace, ignore
if ((searchParams.get('namespace') ?? undefined) !== namespace) {
return resolve(request);
}

let resolved = resolveRequest(requestAndQuery[0], parent, resolve);

// Only add query back if it's a file path (not a core Node module)
if (
path.isAbsolute(resolved)
// Only add query back if it's a file path (not a core Node module)
if (
path.isAbsolute(resolved)

// These two have native loaders which don't support queries
&& !resolved.endsWith('.json')
&& !resolved.endsWith('.node')
) {
resolved += urlSearchParamsStringify(searchParams);
}
) {
resolved += urlSearchParamsStringify(searchParams);
}

return resolved;
};
return resolved;
};
2 changes: 2 additions & 0 deletions src/cjs/api/resolve-implicit-extensions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import path from 'node:path';
import type { NodeError } from '../../types.js';
import { isBarePackageNamePattern } from '../../utils/path-utils.js';
import type { ResolveFilename } from './types.js';

export const implicitlyResolvableExtensions = [
Expand Down Expand Up @@ -34,6 +35,7 @@ export const createImplicitResolver = (
const nodeError = _error as NodeError;
if (
nodeError.code === 'MODULE_NOT_FOUND'
&& !isBarePackageNamePattern.test(request)
) {
const resolved = (
tryExtensions(resolve, request, ...args)
Expand Down
3 changes: 2 additions & 1 deletion src/utils/path-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,5 @@ export const isJsonPattern = /\.json($|\?)/;
export const isDirectoryPattern = /\/(?:$|\?)/;

// Only matches packages names without subpaths (e.g. `foo` but not `foo/bar`)
export const isBarePackageNamePattern = /^(?:@[^/]+\/)?[^/]+$/;
// Back slash included to exclude Windows paths
export const isBarePackageNamePattern = /^(?:@[^/]+\/)?[^/\\]+$/;
148 changes: 75 additions & 73 deletions tests/specs/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,42 +135,84 @@ export default testSuite(({ describe }, node: NodeApis) => {
expect(stdout).toBe('js working\nts working');
});

test('register / unregister', async () => {
await using fixture = await createFixture({
'register.cjs': `
const { register } = require(${JSON.stringify(tsxCjsApiPath)});
try {
require('./file');
} catch {
console.log('Fails as expected');
}
const unregister = register();
const loaded = require('./file');
console.log(loaded.message);
// Remove from cache
const loadedPath = require.resolve('./file');
delete require.cache[loadedPath];
unregister();
try {
require('./file');
} catch {
console.log('Unregistered');
}
`,
...tsFiles,
});
describe('register', ({ test }) => {
test('register / unregister', async () => {
await using fixture = await createFixture({
'register.cjs': `
const { register } = require(${JSON.stringify(tsxCjsApiPath)});
try {
require('./file');
} catch {
console.log('Fails as expected');
}
const unregister = register();
const loaded = require('./file');
console.log(loaded.message);
// Remove from cache
const loadedPath = require.resolve('./file');
delete require.cache[loadedPath];
unregister();
try {
require('./file');
} catch {
console.log('Unregistered');
}
`,
...tsFiles,
});

const { stdout } = await execaNode(fixture.getPath('register.cjs'), [], {
nodePath: node.path,
nodeOptions: [],
const { stdout } = await execaNode(fixture.getPath('register.cjs'), [], {
nodePath: node.path,
nodeOptions: [],
});

expect(stdout).toBe('Fails as expected\nfoo bar json file.ts\nUnregistered');
});

expect(stdout).toBe('Fails as expected\nfoo bar json file.ts\nUnregistered');
test('namespace', async () => {
await using fixture = await createFixture({
'require.cjs': `
const { expectErrors } = require('expect-errors');
const path = require('node:path');
const tsx = require(${JSON.stringify(tsxCjsApiPath)});
const api = tsx.register({ namespace: 'abcd' });
expectErrors(
// Loading explicit/resolved file path should be ignored by loader (extensions)
[() => require('./file.ts'), 'SyntaxError'],
// resolver should preserve full file path when ignoring
[() => require('./file.ts?asdf'), "Cannot find module './file.ts?asdf'"]
);
const { message, async } = api.require('./file', __filename);
console.log(message);
async.then(m => console.log(m.default));
api.require('./tsx?query=1', __filename);
api.require('./jsx', __filename);
api.require('./dir?query=3', __filename);
`,
...tsFiles,

'tsx.tsx': 'console.log(\'tsx\');',
'jsx.jsx': 'console.log(\'jsx\');',
'dir/index.jsx': 'console.log(\'dir\');',
});

const { stdout } = await execaNode(fixture.getPath('require.cjs'), [], {
nodePath: node.path,
nodeOptions: [],
});

expect(stdout).toBe('foo bar json file.ts\ntsx\njsx\ndir\nasync');
});
});

describe('tsx.require()', ({ test }) => {
Expand Down Expand Up @@ -265,46 +307,6 @@ export default testSuite(({ describe }, node: NodeApis) => {

expect(stdout).toBe('foo bar json file.ts\nfoo bar json file.ts\nfoo bar json file.ts\nUnregistered');
});

test('namespace', async () => {
await using fixture = await createFixture({
'require.cjs': `
const { expectErrors } = require('expect-errors');
const path = require('node:path');
const tsx = require(${JSON.stringify(tsxCjsApiPath)});
const api = tsx.register({ namespace: 'abcd' });
expectErrors(
// Loading explicit/resolved file path should be ignored by loader (extensions)
[() => require('./file.ts'), 'SyntaxError'],
// resolver should preserve full file path when ignoring
[() => require('./file.ts?asdf'), "Cannot find module './file.ts?asdf'"]
);
const { message, async } = api.require('./file', __filename);
console.log(message);
async.then(m => console.log(m.default));
api.require('./tsx?query=1', __filename);
api.require('./jsx', __filename);
api.require('./dir?query=3', __filename);
`,
...tsFiles,

'tsx.tsx': 'console.log(\'tsx\');',
'jsx.jsx': 'console.log(\'jsx\');',
'dir/index.jsx': 'console.log(\'dir\');',
});

const { stdout } = await execaNode(fixture.getPath('require.cjs'), [], {
nodePath: node.path,
nodeOptions: [],
});

expect(stdout).toBe('foo bar json file.ts\ntsx\njsx\ndir\nasync');
});
});
});

Expand Down

0 comments on commit ae2a1bc

Please sign in to comment.