Skip to content

Commit 010458d

Browse files
joyeecheungtargos
authored andcommitted
esm: populate separate cache for require(esm) in imported CJS
Otherwise if the ESM happens to be cached separately by the ESM loader before it gets loaded with `require(esm)` from within an imported CJS file (which uses a re-invented require() with a couple of quirks, including a separate cache), it won't be able to load the esm properly from the cache. PR-URL: #59679 Refs: #59666 Refs: #52697 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 794ca35 commit 010458d

File tree

13 files changed

+239
-78
lines changed

13 files changed

+239
-78
lines changed

lib/internal/modules/cjs/loader.js

Lines changed: 80 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ const {
6969
module_export_names_private_symbol,
7070
module_circular_visited_private_symbol,
7171
module_export_private_symbol,
72-
module_parent_private_symbol,
72+
module_first_parent_private_symbol,
73+
module_last_parent_private_symbol,
7374
},
7475
isInsideNodeModules,
7576
} = internalBinding('util');
@@ -94,9 +95,13 @@ const kModuleCircularVisited = module_circular_visited_private_symbol;
9495
*/
9596
const kModuleExport = module_export_private_symbol;
9697
/**
97-
* {@link Module} parent module.
98+
* {@link Module} The first parent module that loads a module with require().
9899
*/
99-
const kModuleParent = module_parent_private_symbol;
100+
const kFirstModuleParent = module_first_parent_private_symbol;
101+
/**
102+
* {@link Module} The last parent module that loads a module with require().
103+
*/
104+
const kLastModuleParent = module_last_parent_private_symbol;
100105

101106
const kIsMainSymbol = Symbol('kIsMainSymbol');
102107
const kIsCachedByESMLoader = Symbol('kIsCachedByESMLoader');
@@ -117,6 +122,7 @@ module.exports = {
117122
findLongestRegisteredExtension,
118123
resolveForCJSWithHooks,
119124
loadSourceForCJSWithHooks: loadSource,
125+
populateCJSExportsFromESM,
120126
wrapSafe,
121127
wrapModuleLoad,
122128
kIsMainSymbol,
@@ -326,7 +332,8 @@ function Module(id = '', parent) {
326332
this.id = id;
327333
this.path = path.dirname(id);
328334
setOwnProperty(this, 'exports', {});
329-
this[kModuleParent] = parent;
335+
this[kFirstModuleParent] = parent;
336+
this[kLastModuleParent] = parent;
330337
updateChildren(parent, this, false);
331338
this.filename = null;
332339
this.loaded = false;
@@ -408,7 +415,7 @@ ObjectDefineProperty(BuiltinModule.prototype, 'isPreloading', isPreloadingDesc);
408415
* @returns {object}
409416
*/
410417
function getModuleParent() {
411-
return this[kModuleParent];
418+
return this[kFirstModuleParent];
412419
}
413420

414421
/**
@@ -418,7 +425,7 @@ function getModuleParent() {
418425
* @returns {void}
419426
*/
420427
function setModuleParent(value) {
421-
this[kModuleParent] = value;
428+
this[kFirstModuleParent] = value;
422429
}
423430

424431
let debug = debuglog('module', (fn) => {
@@ -998,7 +1005,7 @@ function getExportsForCircularRequire(module) {
9981005
const requiredESM = module[kRequiredModuleSymbol];
9991006
if (requiredESM && requiredESM.getStatus() !== kEvaluated) {
10001007
let message = `Cannot require() ES Module ${module.id} in a cycle.`;
1001-
const parent = module[kModuleParent];
1008+
const parent = module[kLastModuleParent];
10021009
if (parent) {
10031010
message += ` (from ${parent.filename})`;
10041011
}
@@ -1279,6 +1286,8 @@ Module._load = function(request, parent, isMain) {
12791286
// load hooks for the module keyed by the (potentially customized) filename.
12801287
module[kURL] = url;
12811288
module[kFormat] = format;
1289+
} else {
1290+
module[kLastModuleParent] = parent;
12821291
}
12831292

12841293
if (parent !== undefined) {
@@ -1398,7 +1407,8 @@ Module._resolveFilename = function(request, parent, isMain, options) {
13981407
const requireStack = [];
13991408
for (let cursor = parent;
14001409
cursor;
1401-
cursor = cursor[kModuleParent]) {
1410+
// TODO(joyeecheung): it makes more sense to use kLastModuleParent here.
1411+
cursor = cursor[kFirstModuleParent]) {
14021412
ArrayPrototypePush(requireStack, cursor.filename || cursor.id);
14031413
}
14041414
let message = `Cannot find module '${request}'`;
@@ -1515,7 +1525,7 @@ function loadESMFromCJS(mod, filename, format, source) {
15151525
// ESM won't be accessible via process.mainModule.
15161526
setOwnProperty(process, 'mainModule', undefined);
15171527
} else {
1518-
const parent = mod[kModuleParent];
1528+
const parent = mod[kLastModuleParent];
15191529

15201530
requireModuleWarningMode ??= getOptionValue('--trace-require-module');
15211531
if (requireModuleWarningMode) {
@@ -1565,54 +1575,66 @@ function loadESMFromCJS(mod, filename, format, source) {
15651575
wrap,
15661576
namespace,
15671577
} = cascadedLoader.importSyncForRequire(mod, filename, source, isMain, parent);
1568-
// Tooling in the ecosystem have been using the __esModule property to recognize
1569-
// transpiled ESM in consuming code. For example, a 'log' package written in ESM:
1570-
//
1571-
// export default function log(val) { console.log(val); }
1572-
//
1573-
// Can be transpiled as:
1574-
//
1575-
// exports.__esModule = true;
1576-
// exports.default = function log(val) { console.log(val); }
1577-
//
1578-
// The consuming code may be written like this in ESM:
1579-
//
1580-
// import log from 'log'
1581-
//
1582-
// Which gets transpiled to:
1583-
//
1584-
// const _mod = require('log');
1585-
// const log = _mod.__esModule ? _mod.default : _mod;
1586-
//
1587-
// So to allow transpiled consuming code to recognize require()'d real ESM
1588-
// as ESM and pick up the default exports, we add a __esModule property by
1589-
// building a source text module facade for any module that has a default
1590-
// export and add .__esModule = true to the exports. This maintains the
1591-
// enumerability of the re-exported names and the live binding of the exports,
1592-
// without incurring a non-trivial per-access overhead on the exports.
1593-
//
1594-
// The source of the facade is defined as a constant per-isolate property
1595-
// required_module_default_facade_source_string, which looks like this
1596-
//
1597-
// export * from 'original';
1598-
// export { default } from 'original';
1599-
// export const __esModule = true;
1600-
//
1601-
// And the 'original' module request is always resolved by
1602-
// createRequiredModuleFacade() to `wrap` which is a ModuleWrap wrapping
1603-
// over the original module.
1604-
1605-
// We don't do this to modules that are marked as CJS ESM or that
1606-
// don't have default exports to avoid the unnecessary overhead.
1607-
// If __esModule is already defined, we will also skip the extension
1608-
// to allow users to override it.
1609-
if (ObjectHasOwn(namespace, 'module.exports')) {
1610-
mod.exports = namespace['module.exports'];
1611-
} else if (!ObjectHasOwn(namespace, 'default') || ObjectHasOwn(namespace, '__esModule')) {
1612-
mod.exports = namespace;
1613-
} else {
1614-
mod.exports = createRequiredModuleFacade(wrap);
1615-
}
1578+
1579+
populateCJSExportsFromESM(mod, wrap, namespace);
1580+
}
1581+
}
1582+
1583+
/**
1584+
* Populate the exports of a CJS module entry from an ESM module's namespace object for
1585+
* require(esm).
1586+
* @param {Module} mod CJS module instance
1587+
* @param {ModuleWrap} wrap ESM ModuleWrap instance.
1588+
* @param {object} namespace The ESM namespace object.
1589+
*/
1590+
function populateCJSExportsFromESM(mod, wrap, namespace) {
1591+
// Tooling in the ecosystem have been using the __esModule property to recognize
1592+
// transpiled ESM in consuming code. For example, a 'log' package written in ESM:
1593+
//
1594+
// export default function log(val) { console.log(val); }
1595+
//
1596+
// Can be transpiled as:
1597+
//
1598+
// exports.__esModule = true;
1599+
// exports.default = function log(val) { console.log(val); }
1600+
//
1601+
// The consuming code may be written like this in ESM:
1602+
//
1603+
// import log from 'log'
1604+
//
1605+
// Which gets transpiled to:
1606+
//
1607+
// const _mod = require('log');
1608+
// const log = _mod.__esModule ? _mod.default : _mod;
1609+
//
1610+
// So to allow transpiled consuming code to recognize require()'d real ESM
1611+
// as ESM and pick up the default exports, we add a __esModule property by
1612+
// building a source text module facade for any module that has a default
1613+
// export and add .__esModule = true to the exports. This maintains the
1614+
// enumerability of the re-exported names and the live binding of the exports,
1615+
// without incurring a non-trivial per-access overhead on the exports.
1616+
//
1617+
// The source of the facade is defined as a constant per-isolate property
1618+
// required_module_default_facade_source_string, which looks like this
1619+
//
1620+
// export * from 'original';
1621+
// export { default } from 'original';
1622+
// export const __esModule = true;
1623+
//
1624+
// And the 'original' module request is always resolved by
1625+
// createRequiredModuleFacade() to `wrap` which is a ModuleWrap wrapping
1626+
// over the original module.
1627+
1628+
// We don't do this to modules that are marked as CJS ESM or that
1629+
// don't have default exports to avoid the unnecessary overhead.
1630+
// If __esModule is already defined, we will also skip the extension
1631+
// to allow users to override it.
1632+
if (ObjectHasOwn(namespace, 'module.exports')) {
1633+
mod.exports = namespace['module.exports'];
1634+
} else if (!ObjectHasOwn(namespace, 'default') || ObjectHasOwn(namespace, '__esModule')) {
1635+
mod.exports = namespace;
1636+
} else {
1637+
mod.exports = createRequiredModuleFacade(wrap);
16161638
}
16171639
}
16181640

@@ -1805,7 +1827,7 @@ function reconstructErrorStack(err, parentPath, parentSource) {
18051827
*/
18061828
function getRequireESMError(mod, pkg, content, filename) {
18071829
// This is an error path because `require` of a `.js` file in a `"type": "module"` scope is not allowed.
1808-
const parent = mod[kModuleParent];
1830+
const parent = mod[kFirstModuleParent];
18091831
const parentPath = parent?.filename;
18101832
const packageJsonPath = pkg?.path;
18111833
const usesEsm = containsModuleSyntax(content, filename);

lib/internal/modules/esm/loader.js

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const {
1717
const {
1818
kIsExecuting,
1919
kRequiredModuleSymbol,
20+
Module: CJSModule,
2021
} = require('internal/modules/cjs/loader');
2122
const { imported_cjs_symbol } = internalBinding('symbols');
2223

@@ -91,13 +92,18 @@ function newLoadCache() {
9192
return new LoadCache();
9293
}
9394

95+
let _translators;
96+
function lazyLoadTranslators() {
97+
_translators ??= require('internal/modules/esm/translators');
98+
return _translators;
99+
}
100+
94101
/**
95102
* Lazy-load translators to avoid potentially unnecessary work at startup (ex if ESM is not used).
96103
* @returns {import('./translators.js').Translators}
97104
*/
98105
function getTranslators() {
99-
const { translators } = require('internal/modules/esm/translators');
100-
return translators;
106+
return lazyLoadTranslators().translators;
101107
}
102108

103109
/**
@@ -506,7 +512,7 @@ class ModuleLoader {
506512

507513
const { source } = loadResult;
508514
const isMain = (parentURL === undefined);
509-
const wrap = this.#translate(url, finalFormat, source, isMain);
515+
const wrap = this.#translate(url, finalFormat, source, parentURL);
510516
assert(wrap instanceof ModuleWrap, `Translator used for require(${url}) should not be async`);
511517

512518
if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
@@ -542,18 +548,31 @@ class ModuleLoader {
542548
* @param {string} format Format of the module to be translated. This is used to find
543549
* matching translators.
544550
* @param {ModuleSource} source Source of the module to be translated.
545-
* @param {boolean} isMain Whether the module to be translated is the entry point.
551+
* @param {string|undefined} parentURL URL of the parent module. Undefined if it's the entry point.
546552
* @returns {ModuleWrap}
547553
*/
548-
#translate(url, format, source, isMain) {
554+
#translate(url, format, source, parentURL) {
549555
this.validateLoadResult(url, format);
550556
const translator = getTranslators().get(format);
551557

552558
if (!translator) {
553559
throw new ERR_UNKNOWN_MODULE_FORMAT(format, url);
554560
}
555561

556-
const result = FunctionPrototypeCall(translator, this, url, source, isMain);
562+
// Populate the CJS cache with a facade for ESM in case subsequent require(esm) is
563+
// looking it up from the cache. The parent module of the CJS cache entry would be the
564+
// first CJS module that loads it with require(). This is an approximation, because
565+
// ESM caches more and it does not get re-loaded and updated every time an `import` is
566+
// encountered, unlike CJS require(), and we only use the parent entry to provide
567+
// more information in error messages.
568+
if (format === 'module') {
569+
const parentFilename = urlToFilename(parentURL);
570+
const parent = parentFilename ? CJSModule._cache[parentFilename] : undefined;
571+
const cjsModule = lazyLoadTranslators().cjsEmplaceModuleCacheEntryForURL(url, parent);
572+
debug('cjsEmplaceModuleCacheEntryForURL', url, parent, cjsModule);
573+
}
574+
575+
const result = FunctionPrototypeCall(translator, this, url, source, parentURL === undefined);
557576
assert(result instanceof ModuleWrap);
558577
return result;
559578
}
@@ -563,10 +582,10 @@ class ModuleLoader {
563582
* This is run synchronously, and the translator always return a ModuleWrap synchronously.
564583
* @param {string} url URL of the module to be translated.
565584
* @param {object} loadContext See {@link load}
566-
* @param {boolean} isMain Whether the module to be translated is the entry point.
585+
* @param {string|undefined} parentURL URL of the parent module. Undefined if it's the entry point.
567586
* @returns {ModuleWrap}
568587
*/
569-
loadAndTranslateForRequireInImportedCJS(url, loadContext, isMain) {
588+
loadAndTranslateForRequireInImportedCJS(url, loadContext, parentURL) {
570589
const { format: formatFromLoad, source } = this.#loadSync(url, loadContext);
571590

572591
if (formatFromLoad === 'wasm') { // require(wasm) is not supported.
@@ -587,7 +606,7 @@ class ModuleLoader {
587606
finalFormat = 'require-commonjs-typescript';
588607
}
589608

590-
const wrap = this.#translate(url, finalFormat, source, isMain);
609+
const wrap = this.#translate(url, finalFormat, source, parentURL);
591610
assert(wrap instanceof ModuleWrap, `Translator used for require(${url}) should not be async`);
592611
return wrap;
593612
}
@@ -597,13 +616,13 @@ class ModuleLoader {
597616
* This may be run asynchronously if there are asynchronous module loader hooks registered.
598617
* @param {string} url URL of the module to be translated.
599618
* @param {object} loadContext See {@link load}
600-
* @param {boolean} isMain Whether the module to be translated is the entry point.
619+
* @param {string|undefined} parentURL URL of the parent module. Undefined if it's the entry point.
601620
* @returns {Promise<ModuleWrap>|ModuleWrap}
602621
*/
603-
loadAndTranslate(url, loadContext, isMain) {
622+
loadAndTranslate(url, loadContext, parentURL) {
604623
const maybePromise = this.load(url, loadContext);
605624
const afterLoad = ({ format, source }) => {
606-
return this.#translate(url, format, source, isMain);
625+
return this.#translate(url, format, source, parentURL);
607626
};
608627
if (isPromise(maybePromise)) {
609628
return maybePromise.then(afterLoad);
@@ -630,9 +649,9 @@ class ModuleLoader {
630649
const isMain = parentURL === undefined;
631650
let moduleOrModulePromise;
632651
if (isForRequireInImportedCJS) {
633-
moduleOrModulePromise = this.loadAndTranslateForRequireInImportedCJS(url, context, isMain);
652+
moduleOrModulePromise = this.loadAndTranslateForRequireInImportedCJS(url, context, parentURL);
634653
} else {
635-
moduleOrModulePromise = this.loadAndTranslate(url, context, isMain);
654+
moduleOrModulePromise = this.loadAndTranslate(url, context, parentURL);
636655
}
637656

638657
const inspectBrk = (

0 commit comments

Comments
 (0)