From dc00a07426f0b2dee9bfe39c766a703eb9b5b880 Mon Sep 17 00:00:00 2001 From: Gus Caplan Date: Thu, 30 Jul 2020 10:28:38 -0500 Subject: [PATCH] Revert "module: disable cjs snapshotting into esm loader" This reverts commit 1fe39f0b4bad8da38e5f02542c176d5999ad3ecb. PR-URL: https://github.com/nodejs/node/pull/34562 Reviewed-By: Bradley Farias Reviewed-By: Guy Bedford --- lib/internal/modules/cjs/loader.js | 32 +++++++++++++++++++++-- lib/internal/modules/esm/loader.js | 6 ++++- lib/internal/modules/esm/translators.js | 34 ++++++++++++++----------- test/es-module/test-esm-snapshot.mjs | 2 +- 4 files changed, 55 insertions(+), 19 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index ab943657a133b4..de24f6c409c498 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -32,6 +32,7 @@ const { ArrayPrototypeJoin, Error, JSONParse, + Map, Number, ObjectCreate, ObjectDefineProperty, @@ -112,6 +113,8 @@ 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 { encodedSepRegEx, packageInternalResolve @@ -371,7 +374,7 @@ function tryPackage(requestPath, exts, isMain, originalPath) { // In order to minimize unnecessary lstat() calls, // this cache is a list of known-real paths. // Set to an empty Map to reset. -const realpathCache = new SafeMap(); +const realpathCache = new Map(); // Check if the file exists and is not a directory // if using --preserve-symlinks and isMain is false, @@ -1115,6 +1118,31 @@ Module.prototype.load = function(filename) { } Module._extensions[extension](this, 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 */) + ); + } }; @@ -1234,7 +1262,7 @@ Module.prototype._compile = function(content, filename) { const exports = this.exports; const thisValue = exports; const module = this; - if (requireDepth === 0) statCache = new SafeMap(); + if (requireDepth === 0) statCache = new Map(); if (inspectorWrapper) { result = inspectorWrapper(compiledWrapper, thisValue, exports, require, module, filename, dirname); diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 62b935de942366..37191f65bf0b7e 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -5,7 +5,8 @@ require('internal/modules/cjs/loader'); const { FunctionPrototypeBind, - ObjectSetPrototypeOf + ObjectSetPrototypeOf, + SafeMap, } = primordials; const { @@ -45,6 +46,9 @@ class Loader { // Registry of loaded modules, akin to `require.cache` this.moduleMap = new ModuleMap(); + // Map of already-loaded CJS modules to use + this.cjsCache = new SafeMap(); + // 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. // The script may evaluate to a function that can be called with a diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js index 45abbae41193f4..bb3528eddde964 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -11,8 +11,6 @@ const { StringPrototypeReplace, } = primordials; -const assert = require('internal/assert'); - let _TYPES = null; function lazyTypes() { if (_TYPES !== null) return _TYPES; @@ -134,21 +132,27 @@ const isWindows = process.platform === 'win32'; const winSepRegEx = /\//g; translators.set('commonjs', function commonjsStrategy(url, isMain) { debug(`Translating CJSModule ${url}`); - let filename = internalURLModule.fileURLToPath(new URL(url)); - if (isWindows) - filename = StringPrototypeReplace(filename, winSepRegEx, '\\'); + 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}`); - let module = CJSModule._cache[filename]; - if (!module || !module.loaded) { - module = new CJSModule(filename); - module.filename = filename; - module.paths = CJSModule._nodeModulePaths(module.path); - CJSModule._cache[filename] = module; - module.load(filename); - assert(module && module.loaded); - } - this.setExport('default', module.exports); + // 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); }); }); diff --git a/test/es-module/test-esm-snapshot.mjs b/test/es-module/test-esm-snapshot.mjs index 6482e3df605252..e2695d20a81747 100644 --- a/test/es-module/test-esm-snapshot.mjs +++ b/test/es-module/test-esm-snapshot.mjs @@ -3,4 +3,4 @@ import '../fixtures/es-modules/esm-snapshot-mutator.js'; import one from '../fixtures/es-modules/esm-snapshot.js'; import assert from 'assert'; -assert.strictEqual(one, 2); +assert.strictEqual(one, 1);