From 9a7c87df37e67dc0629ab9ee816c110ddeb2223c Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 2 Aug 2020 18:03:48 -0700 Subject: [PATCH] module: use cjsCache over esm injection PR-URL: https://github.com/nodejs/node/pull/34605 Reviewed-By: Bradley Farias Reviewed-By: Rich Trott --- lib/internal/modules/cjs/loader.js | 29 ++++------------- lib/internal/modules/esm/loader.js | 4 +-- lib/internal/modules/esm/translators.js | 42 +++++++++---------------- 3 files changed, 22 insertions(+), 53 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 01803211fe0223..13c4589fc377e1 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -112,8 +112,7 @@ const { } = require('internal/util/types'); 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 @@ -1101,29 +1100,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 index 37191f65bf0b7e..110464cbdb1da3 100644 --- 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 { @@ -47,7 +47,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 index bb3528eddde964..bb095446bc27eb 100644 --- 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); }); });