From 76d85fc527d7036be15a8522d0a5180488925788 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 2 Aug 2020 18:03:48 -0700 Subject: [PATCH 1/3] module: use cjsCache over esm injection --- lib/internal/modules/cjs/loader.js | 27 ++++------------ lib/internal/modules/esm/translators.js | 41 ++++++++----------------- 2 files changed, 19 insertions(+), 49 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index de24f6c409c498..2bd8cef20c89ef 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -113,8 +113,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 @@ -1124,25 +1123,11 @@ Module.prototype.load = function(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 === undefined || module.module === undefined || + module.module.getStatus() < kEvaluated) && + !ESMLoader.cjsCache.has(url)) + ESMLoader.cjsCache.set(url, exports); }; diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index bb3528eddde964..4e75eb1882fb1a 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,17 @@ 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; + if (asyncESM.ESMLoader.cjsCache.has(url)) { + exports = asyncESM.ESMLoader.cjsCache.get(url); + asyncESM.ESMLoader.cjsCache.delete(url); + } else { + exports = CJSModule._load(pathname, undefined, isMain); + } + this.setExport('default', exports); }); }); From 7f9861b0c3c4be4455484b1e1c3611c8a7c77e77 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sun, 2 Aug 2020 18:23:33 -0700 Subject: [PATCH 2/3] weakmap approach --- lib/internal/modules/cjs/loader.js | 6 ++---- lib/internal/modules/esm/loader.js | 4 ++-- lib/internal/modules/esm/translators.js | 7 ++++--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 2bd8cef20c89ef..781fcfe33cec98 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1119,15 +1119,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; // Preemptively cache if ((module === undefined || module.module === undefined || module.module.getStatus() < kEvaluated) && - !ESMLoader.cjsCache.has(url)) - ESMLoader.cjsCache.set(url, exports); + !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 4e75eb1882fb1a..bb095446bc27eb 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -131,9 +131,10 @@ translators.set('commonjs', function commonjsStrategy(url, isMain) { debug(`Loading CJSModule ${url}`); const pathname = internalURLModule.fileURLToPath(new URL(url)); let exports; - if (asyncESM.ESMLoader.cjsCache.has(url)) { - exports = asyncESM.ESMLoader.cjsCache.get(url); - asyncESM.ESMLoader.cjsCache.delete(url); + 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); } From 2e0ec3f091c341f7447de7e9c16f7c2171db73b1 Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Sat, 8 Aug 2020 09:36:53 -0700 Subject: [PATCH 3/3] Update lib/internal/modules/cjs/loader.js Co-authored-by: Antoine du Hamel --- lib/internal/modules/cjs/loader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 781fcfe33cec98..821b27fe692d12 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -1122,7 +1122,7 @@ Module.prototype.load = function(filename) { // Create module entry at load time to snapshot exports correctly const exports = this.exports; // Preemptively cache - if ((module === undefined || module.module === undefined || + if ((module?.module === undefined || module.module.getStatus() < kEvaluated) && !ESMLoader.cjsCache.has(this)) ESMLoader.cjsCache.set(this, exports);