From e35902cddb8ae5e0335f4c26588834245ec4cb68 Mon Sep 17 00:00:00 2001 From: Marco Ippolito Date: Thu, 22 Aug 2024 10:48:33 +0200 Subject: [PATCH] module: fix discrepancy between .ts and .js PR-URL: https://github.com/nodejs/node/pull/54461 Fixes: https://github.com/nodejs/node/issues/54457 Reviewed-By: Paolo Insogna Reviewed-By: Matteo Collina Reviewed-By: Chengzhong Wu Reviewed-By: Zeyu "Alex" Yang --- lib/internal/modules/esm/get_format.js | 14 +++--- lib/internal/modules/esm/translators.js | 51 +------------------- lib/internal/modules/helpers.js | 53 +++++++++++++++++++++ test/es-module/test-typescript.mjs | 11 +++++ test/fixtures/typescript/ts/issue-54457.mjs | 4 ++ test/fixtures/typescript/ts/test-require.ts | 7 +++ 6 files changed, 84 insertions(+), 56 deletions(-) create mode 100644 test/fixtures/typescript/ts/issue-54457.mjs create mode 100644 test/fixtures/typescript/ts/test-require.ts diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index a89446df710..0eedf2a7288 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/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. diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 8f88214f558..b1e7b86095c 100644 --- a/lib/internal/modules/esm/translators.js +++ b/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. diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index 729a33e04f3..d805572cc8b 100644 --- a/lib/internal/modules/helpers.js +++ b/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() { diff --git a/test/es-module/test-typescript.mjs b/test/es-module/test-typescript.mjs index 0a713a36e42..3fc753e56c8 100644 --- a/test/es-module/test-typescript.mjs +++ b/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); +}); diff --git a/test/fixtures/typescript/ts/issue-54457.mjs b/test/fixtures/typescript/ts/issue-54457.mjs new file mode 100644 index 00000000000..7394101b61c --- /dev/null +++ b/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); diff --git a/test/fixtures/typescript/ts/test-require.ts b/test/fixtures/typescript/ts/test-require.ts new file mode 100644 index 00000000000..a96db1a6542 --- /dev/null +++ b/test/fixtures/typescript/ts/test-require.ts @@ -0,0 +1,7 @@ +const util = require('node:util'); + +const text = 'Hello, TypeScript!'; + +module.exports = { + text +};