Skip to content

Commit

Permalink
module: move the CJS exports cache to internal/modules/cjs/loader
Browse files Browse the repository at this point in the history
This puts it together with the cjsParseCache and reduces the
circular dependency on the singleton loader, which is the
only place where this cache is stored.

Drive-by: remove always-false module status check because there's
no longer a local module variable after
nodejs#34605 which is now invalid
leftover code at this point and only doesn't throw because
we happen to have a top-level variable called module.
  • Loading branch information
joyeecheung committed Dec 14, 2023
1 parent 429ec83 commit 12234a0
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 16 deletions.
20 changes: 12 additions & 8 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,19 @@ const {

// Map used to store CJS parsing data.
const cjsParseCache = new SafeWeakMap();
/**
* Map of already-loaded CJS modules to use.
*/
const cjsExportsCache = new SafeWeakMap();

// Set first due to cycle with ESM loader functions.
module.exports = {
wrapSafe, Module, cjsParseCache,
get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; },
cjsExportsCache,
cjsParseCache,
initializeCJS,
Module,
wrapSafe,
get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; },
};

const { BuiltinModule } = require('internal/bootstrap/realm');
Expand Down Expand Up @@ -1206,14 +1213,11 @@ Module.prototype.load = function(filename) {
Module._extensions[extension](this, filename);
this.loaded = true;

const cascadedLoader = getCascadedLoader();
// Create module entry at load time to snapshot exports correctly
const exports = this.exports;
// Preemptively cache
if ((module?.module === undefined ||
module.module.getStatus() < kEvaluated) &&
!cascadedLoader.cjsCache.has(this)) {
cascadedLoader.cjsCache.set(this, exports);
// Preemptively cache for ESM loader.
if (!cjsExportsCache.has(this)) {
cjsExportsCache.set(this, exports);
}
};

Expand Down
5 changes: 0 additions & 5 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,6 @@ class ModuleLoader {
*/
#defaultConditions = getDefaultConditions();

/**
* Map of already-loaded CJS modules to use
*/
cjsCache = new SafeWeakMap();

/**
* The index for assigning unique URLs to anonymous module evaluation
*/
Expand Down
7 changes: 4 additions & 3 deletions lib/internal/modules/esm/translators.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const {
const {
Module: CJSModule,
cjsParseCache,
cjsExportsCache,
} = require('internal/modules/cjs/loader');
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
Expand Down Expand Up @@ -308,9 +309,9 @@ function createCJSModuleWrap(url, source, isMain, loadCJS = loadCJSModule) {
}

let exports;
if (asyncESM.esmLoader.cjsCache.has(module)) {
exports = asyncESM.esmLoader.cjsCache.get(module);
asyncESM.esmLoader.cjsCache.delete(module);
if (cjsExportsCache.has(module)) {
exports = cjsExportsCache.get(module);
cjsExportsCache.delete(module);
} else {
({ exports } = module);
}
Expand Down
1 change: 1 addition & 0 deletions lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const {
ObjectPrototypeHasOwnProperty,
SafeMap,
SafeSet,
SafeWeakMap,
StringPrototypeCharCodeAt,
StringPrototypeIncludes,
StringPrototypeSlice,
Expand Down

0 comments on commit 12234a0

Please sign in to comment.