Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: fix discrepancy between .ts and .js #54461

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions lib/internal/modules/esm/get_format.js
Original file line number Diff line number Diff line change
Expand Up @@ -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));
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
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.
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
// Warn about the missing `type` field so that the user can avoid the performance penalty of detection.
Expand Down
51 changes: 2 additions & 49 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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,
Expand All @@ -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');
Expand Down Expand Up @@ -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.
Expand Down
53 changes: 53 additions & 0 deletions lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -381,8 +382,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,
Expand All @@ -391,6 +443,7 @@ module.exports = {
makeRequireFunction,
normalizeReferrerURL,
stripTypeScriptTypes,
stringify,
stripBOM,
toRealPath,
hasStartedUserCJSExecution() {
Expand Down
11 changes: 11 additions & 0 deletions test/es-module/test-typescript.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
4 changes: 4 additions & 0 deletions test/fixtures/typescript/ts/issue-54457.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// https://github.com/nodejs/node/issues/54457
const { text } = await import('./test-require.ts');

console.log(text);
7 changes: 7 additions & 0 deletions test/fixtures/typescript/ts/test-require.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const util = require('node:util');

const text = 'Hello, TypeScript!';

module.exports = {
text
};
Loading