module: fix discrepancy between .ts and .js

PR-URL: #54461
Fixes: #54457
Reviewed-By: Paolo Insogna <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Zeyu "Alex" Yang <[email protected]>

Author: marco-ippolito

lib/internal/modules/esm/get_format.js

@@ -161,14 +161,14 @@ function getFileProtocolModuleFormat(url, context = { __proto__: null }, ignoreE
       default: { // The user did not pass `--experimental-default-type`.
         // `source` is undefined when this is called from `defaultResolve`;
         // but this gets called again from `defaultLoad`/`defaultLoadSync`.
-        let parsedSource;
-        if (source) {
-          const { stripTypeScriptTypes } = require('internal/modules/helpers');
-          parsedSource = stripTypeScriptTypes(source, url);
-        }
+        // Since experimental-strip-types depends on detect-module, we always return null
+        // if source is undefined.
+        if (!source) { return null; }
+        const { stripTypeScriptTypes, stringify } = require('internal/modules/helpers');
+        const stringifiedSource = stringify(source);
+        const parsedSource = stripTypeScriptTypes(stringifiedSource, fileURLToPath(url));
         const detectedFormat = detectModuleFormat(parsedSource, url);
-        // When source is undefined, default to module-typescript.
-        const format = detectedFormat ? `${detectedFormat}-typescript` : 'module-typescript';
+        const format = `${detectedFormat}-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.

lib/internal/modules/esm/translators.js

@@ -18,16 +18,6 @@ const {
   globalThis: { WebAssembly },
 } = primordials;
 
-/** @type {import('internal/util/types')} */
-let _TYPES = null;
-/**
- * Lazily loads and returns the internal/util/types module.
- */
-function lazyTypes() {
-  if (_TYPES !== null) { return _TYPES; }
-  return _TYPES = require('internal/util/types');
-}
-
 const {
   compileFunctionForCJSLoader,
 } = internalBinding('contextify');
@@ -37,7 +27,9 @@ const assert = require('internal/assert');
 const { readFileSync } = require('fs');
 const { dirname, extname, isAbsolute } = require('path');
 const {
+  assertBufferSource,
   loadBuiltinModule,
+  stringify,
   stripTypeScriptTypes,
   stripBOM,
   urlToFilename,
@@ -57,7 +49,6 @@ let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
 const { emitExperimentalWarning, kEmptyObject, setOwnProperty, isWindows } = require('internal/util');
 const {
   ERR_UNKNOWN_BUILTIN_MODULE,
-  ERR_INVALID_RETURN_PROPERTY_VALUE,
 } = require('internal/errors').codes;
 const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
 const moduleWrap = internalBinding('module_wrap');
@@ -107,44 +98,6 @@ function initCJSParseSync() {
 const translators = new SafeMap();
 exports.translators = translators;
 
-let DECODER = null;
-/**
- * Asserts that the given body is a buffer source (either a string, array buffer, or typed array).
- * Throws an error if the body is not a buffer source.
- * @param {string | ArrayBufferView | ArrayBuffer} body - The body to check.
- * @param {boolean} allowString - Whether or not to allow a string as a valid buffer source.
- * @param {string} hookName - The name of the hook being called.
- * @throws {ERR_INVALID_RETURN_PROPERTY_VALUE} If the body is not a buffer source.
- */
-function assertBufferSource(body, allowString, hookName) {
-  if (allowString && typeof body === 'string') {
-    return;
-  }
-  const { isArrayBufferView, isAnyArrayBuffer } = lazyTypes();
-  if (isArrayBufferView(body) || isAnyArrayBuffer(body)) {
-    return;
-  }
-  throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
-    `${allowString ? 'string, ' : ''}array buffer, or typed array`,
-    hookName,
-    'source',
-    body,
-  );
-}
-
-/**
- * Converts a buffer or buffer-like object to a string.
- * @param {string | ArrayBuffer | ArrayBufferView} body - The buffer or buffer-like object to convert to a string.
- * @returns {string} The resulting string.
- */
-function stringify(body) {
-  if (typeof body === 'string') { return body; }
-  assertBufferSource(body, false, 'load');
-  const { TextDecoder } = require('internal/encoding');
-  DECODER = DECODER === null ? new TextDecoder() : DECODER;
-  return DECODER.decode(body);
-}
-
 /**
  * Converts a URL to a file path if the URL protocol is 'file:'.
  * @param {string} url - The URL to convert.

lib/internal/modules/helpers.js

@@ -15,6 +15,7 @@ const {
 } = primordials;
 const {
   ERR_INVALID_ARG_TYPE,
+  ERR_INVALID_RETURN_PROPERTY_VALUE,
 } = require('internal/errors').codes;
 const { BuiltinModule } = require('internal/bootstrap/realm');
 
@@ -384,8 +385,59 @@ function isUnderNodeModules(filename) {
   return ArrayPrototypeIncludes(splitPath, 'node_modules');
 }
 
+/** @type {import('internal/util/types')} */
+let _TYPES = null;
+/**
+ * Lazily loads and returns the internal/util/types module.
+ */
+function lazyTypes() {
+  if (_TYPES !== null) { return _TYPES; }
+  return _TYPES = require('internal/util/types');
+}
+
+
+/**
+ * Asserts that the given body is a buffer source (either a string, array buffer, or typed array).
+ * Throws an error if the body is not a buffer source.
+ * @param {string | ArrayBufferView | ArrayBuffer} body - The body to check.
+ * @param {boolean} allowString - Whether or not to allow a string as a valid buffer source.
+ * @param {string} hookName - The name of the hook being called.
+ * @throws {ERR_INVALID_RETURN_PROPERTY_VALUE} If the body is not a buffer source.
+ */
+function assertBufferSource(body, allowString, hookName) {
+  if (allowString && typeof body === 'string') {
+    return;
+  }
+  const { isArrayBufferView, isAnyArrayBuffer } = lazyTypes();
+  if (isArrayBufferView(body) || isAnyArrayBuffer(body)) {
+    return;
+  }
+  throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
+    `${allowString ? 'string, ' : ''}array buffer, or typed array`,
+    hookName,
+    'source',
+    body,
+  );
+}
+
+let DECODER = null;
+
+/**
+ * Converts a buffer or buffer-like object to a string.
+ * @param {string | ArrayBuffer | ArrayBufferView} body - The buffer or buffer-like object to convert to a string.
+ * @returns {string} The resulting string.
+ */
+function stringify(body) {
+  if (typeof body === 'string') { return body; }
+  assertBufferSource(body, false, 'load');
+  const { TextDecoder } = require('internal/encoding');
+  DECODER = DECODER === null ? new TextDecoder() : DECODER;
+  return DECODER.decode(body);
+}
+
 module.exports = {
   addBuiltinLibsToObject,
+  assertBufferSource,
   getBuiltinModule,
   getCjsConditions,
   initializeCjsConditions,
@@ -394,6 +446,7 @@ module.exports = {
   makeRequireFunction,
   normalizeReferrerURL,
   stripTypeScriptTypes,
+  stringify,
   stripBOM,
   toRealPath,
   hasStartedUserCJSExecution() {

test/es-module/test-typescript.mjs

@@ -313,3 +313,14 @@ test('execute a TypeScript file with CommonJS syntax requiring .mts with require
        match(result.stdout, /Hello, TypeScript!/);
        strictEqual(result.code, 0);
      });
+
+test('execute a JavaScript file importing a cjs TypeScript file', async () => {
+  const result = await spawnPromisified(process.execPath, [
+    '--experimental-strip-types',
+    '--no-warnings',
+    fixtures.path('typescript/ts/issue-54457.mjs'),
+  ]);
+  strictEqual(result.stderr, '');
+  match(result.stdout, /Hello, TypeScript!/);
+  strictEqual(result.code, 0);
+});

test/fixtures/typescript/ts/issue-54457.mjs

@@ -0,0 +1,4 @@
+// https://github.com/nodejs/node/issues/54457
+const { text } = await import('./test-require.ts');
+
+console.log(text);

test/fixtures/typescript/ts/test-require.ts

@@ -0,0 +1,7 @@
+const util = require('node:util');
+
+const text = 'Hello, TypeScript!';
+
+module.exports = {
+    text
+};