From b621ada69aba703a6d432c910cc2e6024755d659 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
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 <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 .../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 9c59b2b5a77218..44abacb41a3430 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 };
@@ -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
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);
     }