From d53a11c797fc3902d71faf5cf015eab85c1af40e Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Mon, 1 Jul 2024 10:19:48 +0900 Subject: [PATCH 1/2] fix: isolated cts import in Node v18 --- src/esm/api/ts-import.ts | 18 +++++++++++++- tests/specs/api.ts | 53 +++++++++++++++++++--------------------- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/esm/api/ts-import.ts b/src/esm/api/ts-import.ts index 02268f4c7..75f813822 100644 --- a/src/esm/api/ts-import.ts +++ b/src/esm/api/ts-import.ts @@ -1,4 +1,5 @@ import { register as cjsRegister } from '../../cjs/api/index.js'; +import { isFeatureSupported, esmLoadReadFile } from '../../utils/node-features.js'; import { register, type TsconfigOptions } from './register.js'; type Options = { @@ -6,6 +7,8 @@ type Options = { onImport?: (url: string) => void; tsconfig?: TsconfigOptions; }; + +const commonjsPattern = /[/\\].+\.(?:cts|cjs)(?:$|\?)/; const tsImport = ( specifier: string, options: string | Options, @@ -22,10 +25,23 @@ const tsImport = ( const namespace = Date.now().toString(); // Keep registered for hanging require() calls - cjsRegister({ + const cjs = cjsRegister({ namespace, }); + /** + * In Node v18, the loader doesn't support reading the CommonJS from + * a data URL, so it can't actually relay the namespace. This is a workaround + * to preemptively determine whether the file is a CommonJS file, and shortcut + * to using the CommonJS loader instead of going through the ESM loader first + */ + if ( + isFeatureSupported(esmLoadReadFile) + && commonjsPattern.test(specifier) + ) { + return Promise.resolve(cjs.require(specifier, parentURL)); + } + /** * We don't want to unregister this after load since there can be child import() calls * that need TS support diff --git a/tests/specs/api.ts b/tests/specs/api.ts index 23b000a5e..337243698 100644 --- a/tests/specs/api.ts +++ b/tests/specs/api.ts @@ -662,8 +662,11 @@ export default testSuite(({ describe }, node: NodeApis) => { // Loads cts vis CJS namespace even if there are no exports await tsImport('./cjs/exports-no.cts', import.meta.url).catch((error) => console.log(error.constructor.name)) - const cjsExport = await tsImport('./cjs/exports-yes.cts', import.meta.url).then(({ cjsReexport, esmSyntax }) => \`\${cjsReexport} \${esmSyntax}\`, err => err.constructor.name); - console.log(cjsExport); + const cts = await tsImport('./cjs/exports-yes.cts', import.meta.url).then(({ cjsReexport, esmSyntax }) => \`\${cjsReexport} \${esmSyntax}\`, err => err.constructor.name); + console.log(cts); + + const cjs = await tsImport('./cjs/reexport.cjs?query', import.meta.url).then(({ cjsReexport, esmSyntax }) => \`\${cjsReexport} \${esmSyntax}\`, err => err.constructor.name); + console.log(cjs); const { message: message2 } = await tsImport('./file.ts?with-query', import.meta.url); console.log(message2); @@ -682,11 +685,15 @@ export default testSuite(({ describe }, node: NodeApis) => { nodeOptions: [], }); - if (node.supports.cjsInterop) { - 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/); - } + expect(stdout).toMatch(new RegExp([ + 'Fails as expected 1', + String.raw`foo bar json file\.ts\?tsx-namespace=\d+`, + 'cts loaded', + 'cjsReexport esm syntax', + 'cjsReexport esm syntax', + String.raw`foo bar json file\.ts\?with-query=&tsx-namespace=\d+`, + 'Fails as expected 2', + ].join(String.raw`\n`))); }); test('commonjs', async () => { @@ -706,10 +713,10 @@ export default testSuite(({ describe }, node: NodeApis) => { const { message: message2 } = await tsImport('./file.ts?with-query', __filename); console.log(message2); - const cts = await tsImport('./cjs/exports-yes.cts', __filename).then(({ cjsReexport, esmSyntax }) => \`\${cjsReexport} \${esmSyntax}\`, err => err.constructor.name); + const cts = await tsImport('./cjs/exports-yes.cts?query', __filename).then(({ cjsReexport, esmSyntax }) => \`\${cjsReexport} \${esmSyntax}\`, err => err.constructor.name); console.log(cts); - const cjs = await tsImport('./cjs/reexport.cjs', __filename).then(({ cjsReexport, esmSyntax }) => \`\${cjsReexport} \${esmSyntax}\`, err => err.constructor.name); + const cjs = await tsImport('./cjs/reexport.cjs?query', __filename).then(({ cjsReexport, esmSyntax }) => \`\${cjsReexport} \${esmSyntax}\`, err => err.constructor.name); console.log(cjs); // Global not polluted @@ -725,25 +732,15 @@ export default testSuite(({ describe }, node: NodeApis) => { nodePath: node.path, nodeOptions: [], }); - if (node.supports.cjsInterop) { - expect(stdout).toMatch(new RegExp( - `${String.raw`Fails as expected 1\n` - + String.raw`foo bar json file\.ts\?tsx-namespace=\d+\n` - + String.raw`foo bar json file\.ts\?with-query=&tsx-namespace=\d+\n` - + String.raw`cjsReexport esm syntax\n` - + String.raw`cjsReexport esm syntax\n` - }Fails as expected 2`, - )); - } else { - expect(stdout).toMatch(new RegExp( - `${String.raw`Fails as expected 1\n` - + String.raw`foo bar json file\.ts\?tsx-namespace=\d+\n` - + String.raw`foo bar json file\.ts\?with-query=&tsx-namespace=\d+\n` - + String.raw`SyntaxError\n` - + String.raw`Error\n` - }Fails as expected 2`, - )); - } + + expect(stdout).toMatch(new RegExp([ + 'Fails as expected 1', + String.raw`foo bar json file\.ts\?tsx-namespace=\d+`, + String.raw`foo bar json file\.ts\?with-query=&tsx-namespace=\d+`, + 'cjsReexport esm syntax', + 'cjsReexport esm syntax', + 'Fails as expected 2', + ].join(String.raw`\n`))); }); test('mts from commonjs', async () => { From b76a0784e41c5b90f865034dc8c9f9ec522279ff Mon Sep 17 00:00:00 2001 From: Hiroki Osame Date: Mon, 1 Jul 2024 11:18:07 +0900 Subject: [PATCH 2/2] wip --- src/esm/api/ts-import.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/esm/api/ts-import.ts b/src/esm/api/ts-import.ts index 75f813822..81c141d93 100644 --- a/src/esm/api/ts-import.ts +++ b/src/esm/api/ts-import.ts @@ -36,7 +36,7 @@ const tsImport = ( * to using the CommonJS loader instead of going through the ESM loader first */ if ( - isFeatureSupported(esmLoadReadFile) + !isFeatureSupported(esmLoadReadFile) && commonjsPattern.test(specifier) ) { return Promise.resolve(cjs.require(specifier, parentURL));