diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 46297fdb2aa611..1f97363b1f9860 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -6,7 +6,6 @@ require('internal/modules/cjs/loader'); const { FunctionPrototypeCall, ObjectSetPrototypeOf, - PromisePrototypeThen, SafeWeakMap, } = primordials; @@ -16,22 +15,21 @@ 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 newResolveCache() { - const { ResolveCache } = require('internal/modules/esm/module_map'); - return new ResolveCache(); -} - function newLoadCache() { const { LoadCache } = require('internal/modules/esm/module_map'); return new LoadCache(); } +function newResolveCache() { + const { ResolveCache } = require('internal/modules/esm/module_map'); + return new ResolveCache(); +} + function getTranslators() { const { translators } = require('internal/modules/esm/translators'); return translators; @@ -68,11 +66,6 @@ class ModuleLoader { */ #defaultConditions = getDefaultConditions(); - /** - * Import cache - */ - #resolveCache = newResolveCache(); - /** * Map of already-loaded CJS modules to use */ @@ -86,7 +79,12 @@ class ModuleLoader { /** * Registry of loaded modules, akin to `require.cache` */ - moduleMap = newLoadCache(); + loadCache = newLoadCache(); + + /** + * Registry of resolved URLs + */ + #resolveCache = newResolveCache(); /** * Methods which translate input code or other information into ES modules @@ -195,7 +193,7 @@ class ModuleLoader { const ModuleJob = require('internal/modules/esm/module_job'); const job = new ModuleJob( this, url, undefined, evalInstance, false, false); - this.moduleMap.set(url, undefined, job); + this.loadCache.set(url, undefined, job); const { module } = await job.run(); return { @@ -225,11 +223,11 @@ class ModuleLoader { getJobFromResolveResult(resolveResult, parentURL, importAssertions) { const { url, format } = resolveResult; const resolvedImportAssertions = resolveResult.importAssertions ?? importAssertions; - let job = this.moduleMap.get(url, resolvedImportAssertions.type); + let job = this.loadCache.get(url, resolvedImportAssertions.type); // CommonJS will set functions for lazy job evaluation. if (typeof job === 'function') { - this.moduleMap.set(url, undefined, job = job()); + this.loadCache.set(url, undefined, job = job()); } if (job === undefined) { @@ -289,15 +287,11 @@ class ModuleLoader { inspectBrk, ); - this.moduleMap.set(url, importAssertions.type, job); + this.loadCache.set(url, importAssertions.type, job); return job; } - cacheStaticImportResult(specifier, parentURL, importAttributes, job) { - this.#resolveCache.setLazy(specifier, parentURL, importAttributes, () => job.module.getNamespace()); - } - /** * This method is usually called indirectly as part of the loading processes. * Use directly with caution. @@ -308,50 +302,8 @@ class ModuleLoader { * @returns {Promise} */ async import(specifier, parentURL, importAssertions) { - const { internalCache, serializedModuleRequest } = - this.#resolveCache.getSerialized(specifier, parentURL, importAssertions); - const removeCache = () => { - // Remove the cache entry if the import fails. - delete internalCache[serializedModuleRequest]; - }; - - // If there is no entry in the cache for this import, create one: - if (internalCache[serializedModuleRequest] == null) { - const result = this.#import(specifier, parentURL, importAssertions); - // Cache the Promise for now: - internalCache[serializedModuleRequest] = result; - PromisePrototypeThen(result, (result) => { - // Once the promise has resolved, we can cache the ModuleJob itself. - internalCache[serializedModuleRequest] = result; - }, removeCache); - return result; - } - - // If the cached value is not a promise, it's already been successfully loaded: - if (!isPromise(internalCache[serializedModuleRequest])) { - return internalCache[serializedModuleRequest]; - } - - // 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 (internalCache[serializedModuleRequest] != null) { - return PromisePrototypeThen(internalCache[serializedModuleRequest], undefined, fallback); - } - // Otherwise create a new cache entry: - const result = this.#import(specifier, parentURL, importAssertions); - internalCache[serializedModuleRequest] = result; - PromisePrototypeThen(result, undefined, removeCache); - return result; - }; - return PromisePrototypeThen(internalCache[serializedModuleRequest], undefined, fallback); - } - - async #import(specifier, parentURL, importAssertions) { - const moduleJob = this.getModuleJob(specifier, parentURL, importAssertions); + const moduleJob = await this.getModuleJob(specifier, parentURL, importAssertions); const { module } = await moduleJob.run(); - - this.#resolveCache.set(specifier, parentURL, importAssertions, moduleJob); return module.getNamespace(); } @@ -373,13 +325,20 @@ class ModuleLoader { * @param {string} [parentURL] The URL path of the module's parent. * @param {ImportAssertions} importAssertions Assertions from the import * statement or expression. - * @returns {Promise<{ format: string, url: URL['href'] }>} + * @returns {{ format: string, url: URL['href'] }} */ resolve(originalSpecifier, parentURL, importAssertions) { if (this.#customizations) { return this.#customizations.resolve(originalSpecifier, parentURL, importAssertions); } - return this.defaultResolve(originalSpecifier, parentURL, importAssertions); + const requestKey = this.#resolveCache.serializeKey(originalSpecifier, importAssertions); + const cachedResult = this.#resolveCache.get(requestKey, parentURL); + if (cachedResult != null) { + return cachedResult; + } + const result = this.defaultResolve(originalSpecifier, parentURL, importAssertions); + this.#resolveCache.set(requestKey, parentURL, result); + return result; } /** diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index bbda3316eeaedd..cb1654812f9d82 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -75,9 +75,7 @@ class ModuleJob { const promises = this.module.link(async (specifier, assertions) => { const job = await this.loader.getModuleJob(specifier, url, assertions); ArrayPrototypePush(dependencyJobs, job); - const result = await job.modulePromise; - this.loader.cacheStaticImportResult(specifier, url, attributes, job); - return result; + return job.modulePromise; }); if (promises !== undefined) diff --git a/lib/internal/modules/esm/module_map.js b/lib/internal/modules/esm/module_map.js index 167b3caa114aaf..56a58dbac1483f 100644 --- a/lib/internal/modules/esm/module_map.js +++ b/lib/internal/modules/esm/module_map.js @@ -5,12 +5,10 @@ const { ArrayPrototypeMap, ArrayPrototypeSort, JSONStringify, - ObjectDefineProperty, ObjectKeys, SafeMap, } = primordials; const { kImplicitAssertType } = require('internal/modules/esm/assert'); -const { setOwnProperty } = require('internal/util'); let debug = require('internal/util/debuglog').debuglog('esm', (fn) => { debug = fn; }); @@ -31,82 +29,54 @@ class ResolveCache extends SafeMap { * * It is exposed to allow more efficient read and overwrite a cache entry. * @param {string} specifier - * @param {string} parentURL * @param {Record} importAssertions - * @returns {{ - * serializedModuleRequest: string, - * internalCache: Record | import('./loader').ModuleExports>, - * }} + * @returns {string} */ - getSerialized(specifier, parentURL, importAssertions) { - let internalCache = super.get(parentURL); - if (internalCache == null) { - super.set(parentURL, internalCache = { __proto__: null }); - } - + serializeKey(specifier, importAssertions) { // To serialize the ModuleRequest (specifier + list of import attributes), // we need to sort the attributes by key, then stringifying, // so that different import statements with the same attributes are always treated // as identical. - const serializedModuleRequest = specifier + ArrayPrototypeJoin( + return specifier + '::' + ArrayPrototypeJoin( ArrayPrototypeMap( ArrayPrototypeSort(ObjectKeys(importAssertions)), (key) => JSONStringify(key) + JSONStringify(importAssertions[key])), ','); - - return { internalCache, serializedModuleRequest }; } /** - * @param {string} specifier + * @param {string} serializedKey * @param {string} parentURL - * @param {Record} importAttributes * @returns {import('./loader').ModuleExports | Promise} */ - get(specifier, parentURL, importAttributes) { - const { internalCache, serializedModuleRequest } = this.getSerialized(specifier, parentURL, importAttributes); - return internalCache[serializedModuleRequest]; - } - - /** - * @param {string} specifier - * @param {string} parentURL - * @param {Record} importAttributes - * @param {import('./loader').ModuleExports | Promise} job - */ - set(specifier, parentURL, importAttributes, job) { - const { internalCache, serializedModuleRequest } = this.getSerialized(specifier, parentURL, importAttributes); - internalCache[serializedModuleRequest] = job; - return this; + get(serializedKey, parentURL) { + let internalCache = super.get(parentURL); + if (internalCache == null) { + super.set(parentURL, internalCache = { __proto__: null }); + } + return internalCache[serializedKey]; } /** - * Adds a cache entry that won't be evaluated until the first time someone tries to read it. - * - * Static imports need to be lazily cached as the link step is done before the - * module exports are actually available. - * @param {string} specifier + * @param {string} serializedKey * @param {string} parentURL - * @param {Record} importAttributes - * @param {() => import('./loader').ModuleExports} getJob + * @param {{ format: string, url: URL['href'] }} result */ - setLazy(specifier, parentURL, importAttributes, getJob) { - const { internalCache, serializedModuleRequest } = this.getSerialized(specifier, parentURL, importAttributes); - ObjectDefineProperty(internalCache, serializedModuleRequest, { - __proto__: null, - configurable: true, - get() { - const val = getJob(); - setOwnProperty(internalCache, serializedModuleRequest, val); - return val; - }, - }); + set(serializedKey, parentURL, result) { + let internalCache = super.get(parentURL); + if (internalCache == null) { + super.set(parentURL, internalCache = { __proto__: null }); + } + internalCache[serializedKey] = result; return this; } - has(specifier, parentURL, importAttributes) { - const { internalCache, serializedModuleRequest } = this.getSerialized(specifier, parentURL, importAttributes); - return serializedModuleRequest in internalCache; + has(serializedKey, parentURL) { + let internalCache = super.get(parentURL); + if (internalCache == null) { + super.set(parentURL, internalCache = { __proto__: null }); + } + return serializedKey in internalCache; } } diff --git a/test/es-module/test-esm-loader-modulemap.js b/test/es-module/test-esm-loader-modulemap.js index 95e998adde335f..860775df0a2ce8 100644 --- 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 { LoadCache } = require('internal/modules/esm/module_map'); +const { LoadCache, ResolveCache } = require('internal/modules/esm/module_map'); const ModuleJob = require('internal/modules/esm/module_job'); const createDynamicModule = require( 'internal/modules/esm/create_dynamic_module'); @@ -98,3 +98,21 @@ const jsonModuleJob = new ModuleJob(loader, stubJsonModule.module, }); }); } + +{ + const resolveMap = new ResolveCache(); + + strictEqual(resolveMap.serializeKey('./file', { __proto__: null }), './file::'); + strictEqual(resolveMap.serializeKey('./file', { __proto__: null, type: 'json' }), './file::"type""json"'); + strictEqual(resolveMap.serializeKey('./file::"type""json"', { __proto__: null }), './file::"type""json"::'); + strictEqual(resolveMap.serializeKey('./file', { __proto__: null, c: 'd', a: 'b' }), './file::"a""b","c""d"'); + strictEqual(resolveMap.serializeKey('./s', { __proto__: null, c: 'd', a: 'b', b: 'c' }), './s::"a""b","b""c","c""d"'); + + resolveMap.set('key1', 'parent1', 1); + resolveMap.set('key2', 'parent1', 2); + resolveMap.set('key2', 'parent2', 3); + + strictEqual(resolveMap.get('key1', 'parent1'), 1); + strictEqual(resolveMap.get('key2', 'parent1'), 2); + strictEqual(resolveMap.get('key2', 'parent2'), 3); +}