Skip to content

Commit

Permalink
fix(cjs): handle re-exports from relative paths
Browse files Browse the repository at this point in the history
  • Loading branch information
privatenumber authored Jun 13, 2024
1 parent 531fafa commit 5166122
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 25 deletions.
2 changes: 1 addition & 1 deletion src/@types/module.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ declare module 'module' {

export function _resolveFilename(
request: string,
parent: Parent,
parent: Parent | undefined,
isMain: boolean,
options?: Record<PropertyKey, unknown>,
): string;
Expand Down
27 changes: 21 additions & 6 deletions src/cjs/api/module-resolve-filename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,36 @@ import { createImplicitResolver } from './resolve-implicit-extensions.js';

const nodeModulesPath = `${path.sep}node_modules${path.sep}`;

export const interopCjsExports = (
const getOriginalFilePath = (
request: string,
) => {
if (!request.startsWith('data:text/javascript,')) {
return request;
return;
}

const queryIndex = request.indexOf('?');
if (queryIndex === -1) {
return request;
return;
}

const searchParams = new URLSearchParams(request.slice(queryIndex + 1));
const filePath = searchParams.get('filePath');
if (filePath) {
return filePath;
}
};

export const interopCjsExports = (
request: string,
) => {
const filePath = getOriginalFilePath(request);
if (filePath) {
// The CJS module cache needs to be updated with the actual path for export parsing to work
// https://github.com/nodejs/node/blob/v22.2.0/lib/internal/modules/esm/translators.js#L338
Module._cache[filePath] = Module._cache[request];
delete Module._cache[request];
request = filePath;
}

return request;
};

Expand All @@ -42,7 +50,7 @@ export const interopCjsExports = (
const resolveTsFilename = (
resolve: SimpleResolve,
request: string,
parent: Module.Parent,
parent: Module.Parent | undefined,
) => {
if (
!(parent?.filename && tsExtensionsPattern.test(parent.filename))
Expand Down Expand Up @@ -73,7 +81,7 @@ const resolveTsFilename = (

const resolveRequest = (
request: string,
parent: Module.Parent,
parent: Module.Parent | undefined,
resolve: SimpleResolve,
) => {
// Support file protocol
Expand Down Expand Up @@ -175,6 +183,13 @@ export const createResolveFilename = (

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]);
Expand Down
1 change: 0 additions & 1 deletion src/esm/hook/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ export const load: LoadHook = async (

const filePathWithNamespace = urlNamespace ? `${filePath}?namespace=${encodeURIComponent(urlNamespace)}` : filePath;

// TODO: re-exports from relative paths cant get detected because of the data URL
loaded.responseURL = `data:text/javascript,${encodeURIComponent(transformed.code)}?filePath=${encodeURIComponent(filePathWithNamespace)}`;
return loaded;
}
Expand Down
45 changes: 28 additions & 17 deletions tests/specs/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,25 @@ const tsFiles = {
export const foo = \`foo \${bar}\` as string
export const async = setTimeout(10).then(() => require('./async')).catch((error) => error);
`,
'exports-no.cts': `
// Supports decorators
const log = (target, key, descriptor) => descriptor;
class Example {
@log
greet() {}
}
console.log("cts loaded" as string)
`,
'exports-yes.cts': 'module.exports.cts = require("./esm-syntax.js").default as string',
'esm-syntax.js': 'export default "cts export"',

cjs: {
'exports-no.cts': `
// Supports decorators
const log = (target, key, descriptor) => descriptor;
class Example {
@log
greet() {}
}
console.log("cts loaded" as string)
`,
'exports-yes.cts': 'module.exports = require("./reexport.cjs") as string',
'esm-syntax.js': 'export const esmSyntax = "esm syntax"',
'reexport.cjs': `
exports.cjsReexport = "cjsReexport";
exports.esmSyntax = require("./esm-syntax.js").esmSyntax;
`,
},

'bar.ts': 'export type A = 1; export { bar } from "pkg"',
'async.ts': 'export default "async"',
'json.json': JSON.stringify({ json: 'json' }),
Expand Down Expand Up @@ -240,7 +248,10 @@ export default testSuite(({ describe }, node: NodeApis) => {
test('cli', async () => {
await using fixture = await createFixture({
'package.json': createPackageJson({ type: 'module' }),
'index.ts': 'import { message } from \'./file\';\n\nconsole.log(message, new Error().stack);',
'index.ts': `
import { message } from "./file";
console.log(message, new Error().stack);
`,
...tsFiles,
});

Expand All @@ -249,7 +260,7 @@ export default testSuite(({ describe }, node: NodeApis) => {
nodeOptions: [node.supports.moduleRegister ? '--import' : '--loader', tsxEsmPath],
});
expect(stdout).toContain('foo bar');
expect(stdout).toContain('index.ts:3:22');
expect(stdout).toContain('index.ts:3:27');
});

if (node.supports.moduleRegister) {
Expand Down Expand Up @@ -526,10 +537,10 @@ export default testSuite(({ describe }, node: NodeApis) => {
console.log(message);
// Loads cts vis CJS namespace even if there are no exports
await tsImport('./exports-no.cts', import.meta.url).catch((error) => console.log(error.constructor.name))
await tsImport('./cjs/exports-no.cts', import.meta.url).catch((error) => console.log(error.constructor.name))
const cts = await tsImport('./exports-yes.cts', import.meta.url).then(m => m.cts, err => err.constructor.name);
console.log(cts);
const cjsExport = await tsImport('./cjs/exports-yes.cts', import.meta.url).then(({ cjsReexport, esmSyntax }) => \`\${cjsReexport} \${esmSyntax}\`, err => err.constructor.name);
console.log(cjsExport);
const { message: message2 } = await tsImport('./file.ts?with-query', import.meta.url);
console.log(message2);
Expand All @@ -549,7 +560,7 @@ export default testSuite(({ describe }, node: NodeApis) => {
});

if (node.supports.cjsInterop) {
expect(stdout).toMatch(/Fails as expected 1\nfoo bar json file\.ts\?tsx-namespace=\d+\ncts loaded\ncts export\nfoo bar json file\.ts\?with-query=&tsx-namespace=\d+\nFails as expected 2/);
expect(stdout).toMatch(/Fails as expected 1\nfoo bar json file\.ts\?tsx-namespace=\d+\ncts loaded\ncjsReexport esm syntax\nfoo bar json file\.ts\?with-query=&tsx-namespace=\d+\nFails as expected 2/);
} else {
expect(stdout).toMatch(/Fails as expected 1\nfoo bar json file\.ts\?tsx-namespace=\d+\nSyntaxError\nSyntaxError\nfoo bar json file\.ts\?with-query=&tsx-namespace=\d+\nFails as expected 2/);
}
Expand Down

0 comments on commit 5166122

Please sign in to comment.