From 5d6c76adee0407252b0785f4b08697e8dd6aea4e Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Sat, 3 Aug 2024 19:15:11 +0200 Subject: [PATCH] module: fix strip-types interaction with detect-module PR-URL: https://github.com/nodejs/node/pull/54164 Reviewed-By: Paolo Insogna Reviewed-By: Jake Yuesong Li Reviewed-By: Matteo Collina Reviewed-By: Zeyu "Alex" Yang Reviewed-By: Yagiz Nizipli --- lib/internal/modules/esm/get_format.js | 3 +- lib/internal/modules/helpers.js | 9 +- lib/internal/modules/run_main.js | 2 +- test/es-module/test-typescript-commonjs.mjs | 2 +- test/es-module/test-typescript-eval.mjs | 30 ++++++- test/es-module/test-typescript-module.mjs | 39 +++++++- test/es-module/test-typescript.mjs | 90 ++++++++++++++++++- .../fixtures/typescript/ts/test-empty-file.ts | 1 + test/fixtures/typescript/ts/test-import-fs.ts | 2 + 9 files changed, 166 insertions(+), 12 deletions(-) create mode 100644 test/fixtures/typescript/ts/test-empty-file.ts create mode 100644 test/fixtures/typescript/ts/test-import-fs.ts diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index a42e1a742a5f56..0a15d5b87d4f95 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -164,7 +164,8 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE const { tsParse } = require('internal/modules/helpers'); const parsedSource = tsParse(source); const detectedFormat = detectModuleFormat(parsedSource, url); - const format = detectedFormat ? `${detectedFormat}-typescript` : 'commonjs-typescript'; + // When source is undefined, default to module-typescript. + const format = detectedFormat ? `${detectedFormat}-typescript` : 'module-typescript'; if (format === 'module-typescript' && foundPackageJson) { // This module has a .js extension, a package.json with no `type` field, and ESM syntax. // Warn about the missing `type` field so that the user can avoid the performance penalty of detection. diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index 2eae0f6cd3f78f..36de471a66a02e 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -307,10 +307,15 @@ function lazyLoadTSParser() { return parseTS; } +/** + * Performs type-stripping to TypeScript source code. + * @param {string} source TypeScript code to parse. + * @returns {string} JavaScript code. + */ function tsParse(source) { - if (!source || typeof source !== 'string') { return; } + if (!source || typeof source !== 'string') { return ''; } const transformSync = lazyLoadTSParser(); - const { code } = transformSync(source); + const { code } = transformSync(source, { __proto__: null, mode: 'strip-only' }); return code; } diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index ee526cc089b20d..1e1a1ea46fc6c1 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -84,7 +84,7 @@ function shouldUseESMLoader(mainPath) { if (getOptionValue('--experimental-strip-types')) { // This ensures that --experimental-default-type=commonjs and .mts files are treated as commonjs if (getOptionValue('--experimental-default-type') === 'commonjs') { return false; } - if (mainPath && StringPrototypeEndsWith(mainPath, '.cts')) { return false; } + if (!mainPath || StringPrototypeEndsWith(mainPath, '.cts')) { return false; } // This will likely change in the future to start with commonjs loader by default if (mainPath && StringPrototypeEndsWith(mainPath, '.mts')) { return true; } } diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs index 40594588f107d7..0ee687e0dcdbe6 100644 --- a/test/es-module/test-typescript-commonjs.mjs +++ b/test/es-module/test-typescript-commonjs.mjs @@ -120,7 +120,7 @@ test('execute a .cts file importing a .mts file export', async () => { strictEqual(result.code, 0); }); -test('expect failure of a .cts file with default type module', async () => { +test('execute a .cts file with default type module', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', '--experimental-default-type=module', // Keeps working with commonjs diff --git a/test/es-module/test-typescript-eval.mjs b/test/es-module/test-typescript-eval.mjs index 196953ef5316fc..120b66a1d26017 100644 --- a/test/es-module/test-typescript-eval.mjs +++ b/test/es-module/test-typescript-eval.mjs @@ -6,7 +6,6 @@ if (!process.config.variables.node_use_amaro) skip('Requires Amaro'); test('eval TypeScript ESM syntax', async () => { const result = await spawnPromisified(process.execPath, [ - '--input-type=module', '--experimental-strip-types', '--eval', `import util from 'node:util' @@ -18,9 +17,22 @@ test('eval TypeScript ESM syntax', async () => { strictEqual(result.code, 0); }); +test('eval TypeScript ESM syntax with input-type module', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--input-type=module', + '--eval', + `import util from 'node:util' + const text: string = 'Hello, TypeScript!' + console.log(util.styleText('red', text));`]); + + match(result.stderr, /Type Stripping is an experimental feature and might change at any time/); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); +}); + test('eval TypeScript CommonJS syntax', async () => { const result = await spawnPromisified(process.execPath, [ - '--input-type=commonjs', '--experimental-strip-types', '--eval', `const util = require('node:util'); @@ -32,6 +44,20 @@ test('eval TypeScript CommonJS syntax', async () => { strictEqual(result.code, 0); }); +test('eval TypeScript CommonJS syntax with input-type commonjs', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--input-type=commonjs', + '--eval', + `const util = require('node:util'); + const text: string = 'Hello, TypeScript!' + console.log(util.styleText('red', text));`, + '--no-warnings']); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.stderr, ''); + strictEqual(result.code, 0); +}); + test('eval TypeScript CommonJS syntax by default', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', diff --git a/test/es-module/test-typescript-module.mjs b/test/es-module/test-typescript-module.mjs index 53c3c4fed0463b..7b75466e02ce9b 100644 --- a/test/es-module/test-typescript-module.mjs +++ b/test/es-module/test-typescript-module.mjs @@ -30,7 +30,6 @@ test('execute an .mts file importing an .mts file', async () => { test('execute an .mts file importing a .ts file', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', - '--experimental-default-type=module', // this should fail '--no-warnings', fixtures.path('typescript/mts/test-import-ts-file.mts'), ]); @@ -40,10 +39,22 @@ test('execute an .mts file importing a .ts file', async () => { strictEqual(result.code, 0); }); -test('execute an .mts file importing a .cts file', async () => { +test('execute an .mts file importing a .ts file with default-type module', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', + '--experimental-default-type=module', '--no-warnings', + fixtures.path('typescript/mts/test-import-ts-file.mts'), + ]); + + strictEqual(result.stderr, ''); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); +}); + +test('execute an .mts file importing a .cts file', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', '--no-warnings', fixtures.path('typescript/mts/test-import-commonjs.mts'), ]); @@ -97,3 +108,27 @@ test('execute a .ts file from node_modules', async () => { strictEqual(result.stdout, ''); strictEqual(result.code, 1); }); + +test('execute an empty .ts file', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--no-warnings', + fixtures.path('typescript/ts/test-empty-file.ts'), + ]); + + strictEqual(result.stderr, ''); + strictEqual(result.stdout, ''); + strictEqual(result.code, 0); +}); + +test('execute .ts file importing a module', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--no-warnings', + fixtures.path('typescript/ts/test-import-fs.ts'), + ]); + + strictEqual(result.stderr, ''); + strictEqual(result.stdout, 'Hello, TypeScript!\n'); + strictEqual(result.code, 0); +}); diff --git a/test/es-module/test-typescript.mjs b/test/es-module/test-typescript.mjs index f8df87b540a4e3..0a713a36e42ace 100644 --- a/test/es-module/test-typescript.mjs +++ b/test/es-module/test-typescript.mjs @@ -17,6 +17,18 @@ test('execute a TypeScript file', async () => { }); test('execute a TypeScript file with imports', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--no-warnings', + fixtures.path('typescript/ts/test-import-foo.ts'), + ]); + + strictEqual(result.stderr, ''); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); +}); + +test('execute a TypeScript file with imports with default-type module', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', '--experimental-default-type=module', @@ -30,6 +42,18 @@ test('execute a TypeScript file with imports', async () => { }); test('execute a TypeScript file with node_modules', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--no-warnings', + fixtures.path('typescript/ts/test-typescript-node-modules.ts'), + ]); + + strictEqual(result.stderr, ''); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); +}); + +test('execute a TypeScript file with node_modules with default-type module', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', '--experimental-default-type=module', @@ -45,7 +69,6 @@ test('execute a TypeScript file with node_modules', async () => { test('expect error when executing a TypeScript file with imports with no extensions', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', - '--experimental-default-type=module', fixtures.path('typescript/ts/test-import-no-extension.ts'), ]); @@ -54,6 +77,19 @@ test('expect error when executing a TypeScript file with imports with no extensi strictEqual(result.code, 1); }); +test('expect error when executing a TypeScript file with imports with no extensions with default-type module', + async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--experimental-default-type=module', + fixtures.path('typescript/ts/test-import-no-extension.ts'), + ]); + + match(result.stderr, /Error \[ERR_MODULE_NOT_FOUND\]:/); + strictEqual(result.stdout, ''); + strictEqual(result.code, 1); + }); + test('expect error when executing a TypeScript file with enum', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', @@ -101,6 +137,17 @@ test('execute a TypeScript file with type definition', async () => { }); test('execute a TypeScript file with type definition but no type keyword', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + fixtures.path('typescript/ts/test-import-no-type-keyword.ts'), + ]); + + match(result.stderr, /does not provide an export named 'MyType'/); + strictEqual(result.stdout, ''); + strictEqual(result.code, 1); +}); + +test('execute a TypeScript file with type definition but no type keyword with default-type modue', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', '--experimental-default-type=module', @@ -124,6 +171,18 @@ test('execute a TypeScript file with CommonJS syntax', async () => { }); test('execute a TypeScript file with ES module syntax', async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--no-warnings', + fixtures.path('typescript/ts/test-module-typescript.ts'), + ]); + + strictEqual(result.stderr, ''); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); +}); + +test('execute a TypeScript file with ES module syntax with default-type module', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', '--experimental-default-type=module', @@ -161,7 +220,6 @@ test('expect stack trace of a TypeScript file to be correct', async () => { test('execute CommonJS TypeScript file from node_modules with require-module', async () => { const result = await spawnPromisified(process.execPath, [ - '--experimental-default-type=module', '--experimental-strip-types', fixtures.path('typescript/ts/test-import-ts-node-modules.ts'), ]); @@ -171,6 +229,19 @@ test('execute CommonJS TypeScript file from node_modules with require-module', a strictEqual(result.code, 1); }); +test('execute CommonJS TypeScript file from node_modules with require-module and default-type module', + async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--experimental-default-type=module', + fixtures.path('typescript/ts/test-import-ts-node-modules.ts'), + ]); + + match(result.stderr, /ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING/); + strictEqual(result.stdout, ''); + strictEqual(result.code, 1); + }); + test('execute a TypeScript file with CommonJS syntax but default type module', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', @@ -220,7 +291,6 @@ test('execute a TypeScript file with CommonJS syntax requiring .mts with require test('execute a TypeScript file with CommonJS syntax requiring .mts with require-module', async () => { const result = await spawnPromisified(process.execPath, [ '--experimental-strip-types', - '--experimental-default-type=commonjs', '--no-warnings', fixtures.path('typescript/ts/test-require-cts.ts'), ]); @@ -229,3 +299,17 @@ test('execute a TypeScript file with CommonJS syntax requiring .mts with require match(result.stdout, /Hello, TypeScript!/); strictEqual(result.code, 0); }); + +test('execute a TypeScript file with CommonJS syntax requiring .mts with require-module with default-type commonjs', + async () => { + const result = await spawnPromisified(process.execPath, [ + '--experimental-strip-types', + '--experimental-default-type=commonjs', + '--no-warnings', + fixtures.path('typescript/ts/test-require-cts.ts'), + ]); + + strictEqual(result.stderr, ''); + match(result.stdout, /Hello, TypeScript!/); + strictEqual(result.code, 0); + }); diff --git a/test/fixtures/typescript/ts/test-empty-file.ts b/test/fixtures/typescript/ts/test-empty-file.ts new file mode 100644 index 00000000000000..8b137891791fe9 --- /dev/null +++ b/test/fixtures/typescript/ts/test-empty-file.ts @@ -0,0 +1 @@ + diff --git a/test/fixtures/typescript/ts/test-import-fs.ts b/test/fixtures/typescript/ts/test-import-fs.ts new file mode 100644 index 00000000000000..032a93fa52eee5 --- /dev/null +++ b/test/fixtures/typescript/ts/test-import-fs.ts @@ -0,0 +1,2 @@ +import fs from 'fs'; +console.log('Hello, TypeScript!');