module: fix strip-types interaction with detect-module

PR-URL: nodejs#54164 Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
marco-ippolito · Aug 5, 2024 · 23d5d60 · 23d5d60
1 parent be983c9
commit 23d5d60
Show file tree

Hide file tree

Showing 10 changed files with 168 additions and 13 deletions.
diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js
@@ -163,8 +163,9 @@ 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';
-        if (format === 'module-typescript' && foundPackageJson) {
+        // When source is undefined, default to module-typescript.
+        const format = detectedFormat ? `${detectedFormat}-typescript` : 'module-typescript';
+        if (format === 'module-typescript') {
           // 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.
           warnTypelessPackageJsonFile(pjsonPath, url);

diff --git 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
@@ -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/src/node_options.cc b/src/node_options.cc
@@ -800,6 +800,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "Experimental type-stripping for TypeScript files.",
             &EnvironmentOptions::experimental_strip_types,
             kAllowedInEnvvar);
+  Implies("--experimental-strip-types", "--experimental-detect-module");
   AddOption("--interactive",
             "always enter the REPL even if stdin does not appear "
             "to be a terminal",

diff --git a/test/es-module/test-typescript-commonjs.mjs b/test/es-module/test-typescript-commonjs.mjs
@@ -118,7 +118,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
@@ -4,7 +4,6 @@ import { test } from 'node:test';
 
 test('eval TypeScript ESM syntax', async () => {
   const result = await spawnPromisified(process.execPath, [
-    '--input-type=module',
     '--experimental-strip-types',
     '--eval',
     `import util from 'node:util'
@@ -16,9 +15,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');
@@ -30,6 +42,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
@@ -28,7 +28,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'),
   ]);
@@ -38,10 +37,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'),
   ]);
@@ -95,3 +106,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
@@ -15,6 +15,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',
@@ -28,6 +40,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',
@@ -43,7 +67,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'),
   ]);
 
@@ -52,6 +75,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',
@@ -99,6 +135,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',
@@ -122,6 +169,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',
@@ -159,7 +218,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'),
   ]);
@@ -169,6 +227,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',
@@ -218,7 +289,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'),
   ]);
@@ -227,3 +297,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
@@ -0,0 +1 @@
+
diff --git a/test/fixtures/typescript/ts/test-import-fs.ts b/test/fixtures/typescript/ts/test-import-fs.ts
@@ -0,0 +1,2 @@
+import fs from 'fs';
+console.log('Hello, TypeScript!');