create ModuleResolveMap class

nodejs · Jul 21, 2023 · 3bf7e02 · 3bf7e02
1 parent 0f1fe00
commit 3bf7e02
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 74 deletions.
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -4,16 +4,9 @@
 require('internal/modules/cjs/loader');
 
 const {
-  ArrayPrototypeJoin,
-  ArrayPrototypeMap,
-  ArrayPrototypeSort,
   FunctionPrototypeCall,
-  JSONStringify,
-  ObjectKeys,
   ObjectSetPrototypeOf,
   PromisePrototypeThen,
-  SafeMap,
-  PromiseResolve,
   SafeWeakMap,
 } = primordials;
 
@@ -23,14 +16,20 @@ const {
 const { getOptionValue } = require('internal/options');
 const { pathToFileURL } = require('internal/url');
 const { emitExperimentalWarning } = require('internal/util');
+const { isPromise } = require('internal/util/types');
 const {
   getDefaultConditions,
 } = require('internal/modules/esm/utils');
 let defaultResolve, defaultLoad, importMetaInitializer;
 
-function newModuleMap() {
-  const ModuleMap = require('internal/modules/esm/module_map');
-  return new ModuleMap();
+function newModuleResolveMap() {
+  const { ModuleResolveMap } = require('internal/modules/esm/module_map');
+  return new ModuleResolveMap();
+}
+
+function newModuleLoadMap() {
+  const { ModuleLoadMap } = require('internal/modules/esm/module_map');
+  return new ModuleLoadMap();
 }
 
 function getTranslators() {
@@ -72,7 +71,7 @@ class ModuleLoader {
   /**
    * Import cache
    */
-  #importCache = new SafeMap();
+  #resolveCache = newModuleResolveMap();
 
   /**
    * Map of already-loaded CJS modules to use
@@ -87,7 +86,7 @@ class ModuleLoader {
   /**
    * Registry of loaded modules, akin to `require.cache`
    */
-  moduleMap = newModuleMap();
+  moduleMap = newModuleLoadMap();
 
   /**
    * Methods which translate input code or other information into ES modules
@@ -295,32 +294,8 @@ class ModuleLoader {
     return job;
   }
 
-  #serializeCache(specifier, parentURL, importAssertions) {
-    let cache = this.#importCache.get(parentURL);
-    let specifierCache;
-    if (cache == null) {
-      this.#importCache.set(parentURL, cache = new SafeMap());
-    } else {
-      specifierCache = cache.get(specifier);
-    }
-
-    if (specifierCache == null) {
-      cache.set(specifier, specifierCache = { __proto__: null });
-    }
-
-    const serializedAttributes = ArrayPrototypeJoin(
-      ArrayPrototypeMap(
-        ArrayPrototypeSort(ObjectKeys(importAssertions)),
-        (key) => JSONStringify(key) + JSONStringify(importAssertions[key])),
-      ',');
-
-    return { specifierCache, serializedAttributes };
-  }
-
-  cacheStatic(specifier, parentURL, importAssertions, result) {
-    const { specifierCache, serializedAttributes } = this.#serializeCache(specifier, parentURL, importAssertions);
-
-    specifierCache[serializedAttributes] = result;
+  cacheStaticImportResult(specifier, parentURL, importAttributes, job) {
+    this.#resolveCache.set(specifier, parentURL, importAttributes, () => job.module.getNamespace());
   }
 
   /**
@@ -333,38 +308,55 @@ class ModuleLoader {
    * @returns {Promise<ModuleExports>}
    */
   async import(specifier, parentURL, importAssertions) {
-    const { specifierCache, serializedAttributes } = this.#serializeCache(specifier, parentURL, importAssertions);
+    const { specifierCache, serializedAttributes } =
+      this.#resolveCache.getSerialized(specifier, parentURL, importAssertions);
     const removeCache = () => {
+      // Remove the cache entry if the import fails.
       delete specifierCache[serializedAttributes];
     };
-    if (specifierCache[serializedAttributes] != null) {
-      if (PromiseResolve(specifierCache[serializedAttributes]) !== specifierCache[serializedAttributes]) {
-        const { module } = await specifierCache[serializedAttributes].run();
-        return module.getNamespace();
-      }
-      const fallback = () => {
-        if (specifierCache[serializedAttributes] != null) {
-          return PromisePrototypeThen(specifierCache[serializedAttributes], undefined, fallback);
-        }
-        const result = this.#import(specifier, parentURL, importAssertions);
+
+    // If there are no cache entry, create one:
+    if (specifierCache[serializedAttributes] == null) {
+      const result = this.#import(specifier, parentURL, importAssertions);
+      // Cache the Promise for now:
+      specifierCache[serializedAttributes] = result;
+      PromisePrototypeThen(result, (result) => {
+        // Once the promise has resolved, we can cache the ModuleJob itself.
         specifierCache[serializedAttributes] = result;
-        PromisePrototypeThen(result, undefined, removeCache);
-        return result;
-      };
-      return PromisePrototypeThen(specifierCache[serializedAttributes], undefined, fallback);
+      }, removeCache);
+      return result;
     }
-    const result = this.#import(specifier, parentURL, importAssertions);
-    specifierCache[serializedAttributes] = result;
-    PromisePrototypeThen(result, undefined, removeCache);
-    return result;
+
+    // If the cache entry is a function, it's a static import that has already been successfully loaded:
+    if (typeof specifierCache[serializedAttributes] === 'function') {
+      return specifierCache[serializedAttributes] = specifierCache[serializedAttributes]();
+    }
+
+    // If the cached value is not a promise, it's already been successfully loaded:
+    if (!isPromise(specifierCache[serializedAttributes])) {
+      return specifierCache[serializedAttributes];
+    }
+
+    // If it's still a promise, we must have a fallback in case it fails:
+    const fallback = () => {
+      // If another fallback has already cached a promise, use this one:
+      if (specifierCache[serializedAttributes] != null) {
+        return PromisePrototypeThen(specifierCache[serializedAttributes], undefined, fallback);
+      }
+      // Otherwise create a new cache entry:
+      const result = this.#import(specifier, parentURL, importAssertions);
+      specifierCache[serializedAttributes] = result;
+      PromisePrototypeThen(result, undefined, removeCache);
+      return result;
+    };
+    return PromisePrototypeThen(specifierCache[serializedAttributes], undefined, fallback);
   }
 
   async #import(specifier, parentURL, importAssertions) {
     const moduleJob = this.getModuleJob(specifier, parentURL, importAssertions);
     const { module } = await moduleJob.run();
 
-    const { specifierCache, serializedAttributes } = this.#serializeCache(specifier, parentURL, importAssertions);
-    specifierCache[serializedAttributes] = moduleJob;
+    this.#resolveCache.set(specifier, parentURL, importAssertions, moduleJob);
     return module.getNamespace();
   }
 

diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js
@@ -76,7 +76,7 @@ class ModuleJob {
         const job = await this.loader.getModuleJob(specifier, url, assertions);
         ArrayPrototypePush(dependencyJobs, job);
         const result = await job.modulePromise;
-        this.loader.cacheStatic(specifier, url, assertions, job);
+        this.loader.cacheStaticImportResult(specifier, url, attributes, job);
         return result;
       });
 

diff --git a/lib/internal/modules/esm/module_map.js b/lib/internal/modules/esm/module_map.js
@@ -1,17 +1,64 @@
 'use strict';
 
-const { kImplicitAssertType } = require('internal/modules/esm/assert');
 const {
+  ArrayPrototypeJoin,
+  ArrayPrototypeMap,
+  ArrayPrototypeSort,
+  JSONStringify,
+  ObjectKeys,
   SafeMap,
 } = primordials;
+const { kImplicitAssertType } = require('internal/modules/esm/assert');
 let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
   debug = fn;
 });
 const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
 const { validateString } = require('internal/validators');
 
+class ModuleResolveMap extends SafeMap {
+  constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
+
+  getSerialized(specifier, parentURL, importAssertions) {
+    let cache = super.get(parentURL);
+    let specifierCache;
+    if (cache == null) {
+      super.set(parentURL, cache = new SafeMap());
+    } else {
+      specifierCache = cache.get(specifier);
+    }
+
+    if (specifierCache == null) {
+      cache.set(specifier, specifierCache = { __proto__: null });
+    }
+
+    const serializedAttributes = ArrayPrototypeJoin(
+      ArrayPrototypeMap(
+        ArrayPrototypeSort(ObjectKeys(importAssertions)),
+        (key) => JSONStringify(key) + JSONStringify(importAssertions[key])),
+      ',');
+
+    return { specifierCache, serializedAttributes };
+  }
+
+  get(specifier, parentURL, importAttributes) {
+    const { specifierCache, serializedAttributes } = this.getSerialized(specifier, parentURL, importAttributes);
+    return specifierCache[serializedAttributes];
+  }
+
+  set(specifier, parentURL, importAttributes, job) {
+    const { specifierCache, serializedAttributes } = this.getSerialized(specifier, parentURL, importAttributes);
+    specifierCache[serializedAttributes] = job;
+    return this;
+  }
+
+  has(specifier, parentURL, importAttributes) {
+    const { specifierCache, serializedAttributes } = this.getSerialized(specifier, parentURL, importAttributes);
+    return serializedAttributes in specifierCache;
+  }
+}
+
 // Tracks the state of the loader-level module cache
-class ModuleMap extends SafeMap {
+class ModuleLoadMap extends SafeMap {
   constructor(i) { super(i); } // eslint-disable-line no-useless-constructor
   get(url, type = kImplicitAssertType) {
     validateString(url, 'url');
@@ -29,7 +76,7 @@ class ModuleMap extends SafeMap {
     }
     debug(`Storing ${url} (${
       type === kImplicitAssertType ? 'implicit type' : type
-    }) in ModuleMap`);
+    }) in ModuleLoadMap`);
     const cachedJobsForUrl = super.get(url) ?? { __proto__: null };
     cachedJobsForUrl[type] = job;
     return super.set(url, cachedJobsForUrl);
@@ -40,4 +87,8 @@ class ModuleMap extends SafeMap {
     return super.get(url)?.[type] !== undefined;
   }
 }
-module.exports = ModuleMap;
+
+module.exports = {
+  ModuleLoadMap,
+  ModuleResolveMap,
+};
diff --git a/test/es-module/test-esm-loader-modulemap.js b/test/es-module/test-esm-loader-modulemap.js
@@ -5,7 +5,7 @@ require('../common');
 
 const { strictEqual, throws } = require('assert');
 const { createModuleLoader } = require('internal/modules/esm/loader');
-const ModuleMap = require('internal/modules/esm/module_map');
+const { ModuleLoadMap } = require('internal/modules/esm/module_map');
 const ModuleJob = require('internal/modules/esm/module_job');
 const createDynamicModule = require(
   'internal/modules/esm/create_dynamic_module');
@@ -24,11 +24,11 @@ const jsonModuleJob = new ModuleJob(loader, stubJsonModule.module,
                                     () => new Promise(() => {}));
 
 
-// ModuleMap.set and ModuleMap.get store and retrieve module jobs for a
-// specified url/type tuple; ModuleMap.has correctly reports whether such jobs
+// ModuleLoadMap.set and ModuleLoadMap.get store and retrieve module jobs for a
+// specified url/type tuple; ModuleLoadMap.has correctly reports whether such jobs
 // are stored in the map.
 {
-  const moduleMap = new ModuleMap();
+  const moduleMap = new ModuleLoadMap();
 
   moduleMap.set(jsModuleDataUrl, undefined, jsModuleJob);
   moduleMap.set(jsonModuleDataUrl, 'json', jsonModuleJob);
@@ -50,10 +50,10 @@ const jsonModuleJob = new ModuleJob(loader, stubJsonModule.module,
   strictEqual(moduleMap.has(jsonModuleDataUrl, 'unknown'), false);
 }
 
-// ModuleMap.get, ModuleMap.has and ModuleMap.set should only accept string
+// ModuleLoadMap.get, ModuleLoadMap.has and ModuleLoadMap.set should only accept string
 // values as url argument.
 {
-  const moduleMap = new ModuleMap();
+  const moduleMap = new ModuleLoadMap();
 
   const errorObj = {
     code: 'ERR_INVALID_ARG_TYPE',
@@ -68,10 +68,10 @@ const jsonModuleJob = new ModuleJob(loader, stubJsonModule.module,
   });
 }
 
-// ModuleMap.get, ModuleMap.has and ModuleMap.set should only accept string
+// ModuleLoadMap.get, ModuleLoadMap.has and ModuleLoadMap.set should only accept string
 // values (or the kAssertType symbol) as type argument.
 {
-  const moduleMap = new ModuleMap();
+  const moduleMap = new ModuleLoadMap();
 
   const errorObj = {
     code: 'ERR_INVALID_ARG_TYPE',
@@ -86,9 +86,9 @@ const jsonModuleJob = new ModuleJob(loader, stubJsonModule.module,
   });
 }
 
-// ModuleMap.set should only accept ModuleJob values as job argument.
+// ModuleLoadMap.set should only accept ModuleJob values as job argument.
 {
-  const moduleMap = new ModuleMap();
+  const moduleMap = new ModuleLoadMap();
 
   [{}, [], true, 1].forEach((value) => {
     throws(() => moduleMap.set('', undefined, value), {