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 #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: #51157 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jan 2, 2024 · b621ada · b621ada
1 parent 9bfd84c
commit b621ada
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 19 deletions.
diff --git 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
@@ -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 };
@@ -1206,14 +1212,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
@@ -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
@@ -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);
     }