module: use cjsCache over esm injection

PR-URL: #34605 Backport-PR-URL: #35385 Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
nodejs · Oct 1, 2020 · 3ad146d · 3ad146d
1 parent 00aa935
commit 3ad146d
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 53 deletions.
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -104,8 +104,7 @@ const {
 } = require('internal/constants');
 
 const asyncESM = require('internal/process/esm_loader');
-const ModuleJob = require('internal/modules/esm/module_job');
-const { ModuleWrap, kInstantiated } = internalBinding('module_wrap');
+const { kEvaluated } = internalBinding('module_wrap');
 const {
   encodedSepRegEx,
   packageInternalResolve
@@ -1039,29 +1038,13 @@ Module.prototype.load = function(filename) {
   this.loaded = true;
 
   const ESMLoader = asyncESM.ESMLoader;
-  const url = `${pathToFileURL(filename)}`;
-  const module = ESMLoader.moduleMap.get(url);
   // Create module entry at load time to snapshot exports correctly
   const exports = this.exports;
-  // Called from cjs translator
-  if (module !== undefined && module.module !== undefined) {
-    if (module.module.getStatus() >= kInstantiated)
-      module.module.setExport('default', exports);
-  } else {
-    // Preemptively cache
-    // We use a function to defer promise creation for async hooks.
-    ESMLoader.moduleMap.set(
-      url,
-      // Module job creation will start promises.
-      // We make it a function to lazily trigger those promises
-      // for async hooks compatibility.
-      () => new ModuleJob(ESMLoader, url, () =>
-        new ModuleWrap(url, undefined, ['default'], function() {
-          this.setExport('default', exports);
-        })
-      , false /* isMain */, false /* inspectBrk */)
-    );
-  }
+  // Preemptively cache
+  if ((module?.module === undefined ||
+       module.module.getStatus() < kEvaluated) &&
+      !ESMLoader.cjsCache.has(this))
+    ESMLoader.cjsCache.set(this, exports);
 };
 
 

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -6,7 +6,7 @@ require('internal/modules/cjs/loader');
 const {
   FunctionPrototypeBind,
   ObjectSetPrototypeOf,
-  SafeMap,
+  SafeWeakMap,
 } = primordials;
 
 const {
@@ -52,7 +52,7 @@ class Loader {
     this.moduleMap = new ModuleMap();
 
     // Map of already-loaded CJS modules to use
-    this.cjsCache = new SafeMap();
+    this.cjsCache = new SafeWeakMap();
 
     // This hook is called before the first root module is imported. It's a
     // function that returns a piece of code that runs as a sloppy-mode script.

diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -48,6 +48,8 @@ const experimentalImportMetaResolve =
 const translators = new SafeMap();
 exports.translators = translators;
 
+const asyncESM = require('internal/process/esm_loader');
+
 let DECODER = null;
 function assertBufferSource(body, allowString, hookName) {
   if (allowString && typeof body === 'string') {
@@ -80,21 +82,14 @@ function errPath(url) {
   return url;
 }
 
-let esmLoader;
 async function importModuleDynamically(specifier, { url }) {
-  if (!esmLoader) {
-    esmLoader = require('internal/process/esm_loader').ESMLoader;
-  }
-  return esmLoader.import(specifier, url);
+  return asyncESM.ESMLoader.import(specifier, url);
 }
 
 function createImportMetaResolve(defaultParentUrl) {
   return async function resolve(specifier, parentUrl = defaultParentUrl) {
-    if (!esmLoader) {
-      esmLoader = require('internal/process/esm_loader').ESMLoader;
-    }
     return PromisePrototypeCatch(
-      esmLoader.resolve(specifier, parentUrl),
+      asyncESM.ESMLoader.resolve(specifier, parentUrl),
       (error) => (
         error.code === 'ERR_UNSUPPORTED_DIR_IMPORT' ?
           error.url : PromiseReject(error))
@@ -132,27 +127,18 @@ const isWindows = process.platform === 'win32';
 const winSepRegEx = /\//g;
 translators.set('commonjs', function commonjsStrategy(url, isMain) {
   debug(`Translating CJSModule ${url}`);
-  const pathname = internalURLModule.fileURLToPath(new URL(url));
-  const cached = this.cjsCache.get(url);
-  if (cached) {
-    this.cjsCache.delete(url);
-    return cached;
-  }
-  const module = CJSModule._cache[
-    isWindows ? StringPrototypeReplace(pathname, winSepRegEx, '\\') : pathname
-  ];
-  if (module && module.loaded) {
-    const exports = module.exports;
-    return new ModuleWrap(url, undefined, ['default'], function() {
-      this.setExport('default', exports);
-    });
-  }
   return new ModuleWrap(url, undefined, ['default'], function() {
     debug(`Loading CJSModule ${url}`);
-    // We don't care about the return val of _load here because Module#load
-    // will handle it for us by checking the loader registry and filling the
-    // exports like above
-    CJSModule._load(pathname, undefined, isMain);
+    const pathname = internalURLModule.fileURLToPath(new URL(url));
+    let exports;
+    const cachedModule = CJSModule._cache[pathname];
+    if (cachedModule && asyncESM.ESMLoader.cjsCache.has(cachedModule)) {
+      exports = asyncESM.ESMLoader.cjsCache.get(cachedModule);
+      asyncESM.ESMLoader.cjsCache.delete(cachedModule);
+    } else {
+      exports = CJSModule._load(pathname, undefined, isMain);
+    }
+    this.setExport('default', exports);
   });
 });