From d8da197f86dd1c76fa3526a46b30f5ef0ed7780c Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 14 Dec 2023 17:25:07 +0100 Subject: [PATCH] module: move the CJS exports cache to internal/modules/cjs/loader 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 https://github.com/nodejs/node/pull/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. PR-URL: https://github.com/nodejs/node/pull/51157 Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- .../bootstrap/switches/is_main_thread.js | 3 ++- lib/internal/modules/cjs/loader.js | 21 +++++++++++-------- lib/internal/modules/esm/loader.js | 6 ------ lib/internal/modules/esm/translators.js | 7 ++++--- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/lib/internal/bootstrap/switches/is_main_thread.js b/lib/internal/bootstrap/switches/is_main_thread.js index 8707bc7daaa616..3a0a2eddf92a9e 100644 --- a/lib/internal/bootstrap/switches/is_main_thread.js +++ b/lib/internal/bootstrap/switches/is_main_thread.js @@ -291,10 +291,11 @@ rawMethods.resetStdioForTesting = function() { require('fs'); require('util'); require('url'); // eslint-disable-line no-restricted-modules - +internalBinding('module_wrap'); require('internal/modules/cjs/loader'); require('internal/modules/esm/utils'); require('internal/vm/module'); + // Needed to refresh the time origin. require('internal/perf/utils'); // Needed to register the async hooks. diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index b077ee386bb40e..9206d7d547fe4f 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -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'); @@ -150,7 +157,6 @@ const { isProxy, } = require('internal/util/types'); -const { kEvaluated } = internalBinding('module_wrap'); const isWindows = process.platform === 'win32'; const relativeResolveCache = { __proto__: null }; @@ -1207,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); } }; diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 0694c7ef2b902d..c0e3cdb36e1c02 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -11,7 +11,6 @@ const { JSONStringify, ObjectSetPrototypeOf, RegExpPrototypeSymbolReplace, - SafeWeakMap, encodeURIComponent, hardenRegExp, } = primordials; @@ -86,11 +85,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 */ diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 47906cff86a225..4ecf862be9ea7b 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -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) => { @@ -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); }