From 1fe39f0b4bad8da38e5f02542c176d5999ad3ecb Mon Sep 17 00:00:00 2001 From: Guy Bedford Date: Tue, 21 Jul 2020 13:40:49 -0700 Subject: [PATCH] module: disable cjs snapshotting into esm loader PR-URL: https://github.com/nodejs/node/pull/34467 Reviewed-By: Bradley Farias Reviewed-By: Geoffrey Booth --- 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, 19 insertions(+), 55 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index de24f6c409c498..ab943657a133b4 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -32,7 +32,6 @@ const { ArrayPrototypeJoin, Error, JSONParse, - Map, Number, ObjectCreate, ObjectDefineProperty, @@ -113,8 +112,6 @@ 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 @@ -374,7 +371,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 Map(); +const realpathCache = new SafeMap(); // Check if the file exists and is not a directory // if using --preserve-symlinks and isMain is false, @@ -1118,31 +1115,6 @@ 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 */) - ); - } }; @@ -1262,7 +1234,7 @@ Module.prototype._compile = function(content, filename) { const exports = this.exports; const thisValue = exports; const module = this; - if (requireDepth === 0) statCache = new Map(); + if (requireDepth === 0) statCache = new SafeMap(); 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 37191f65bf0b7e..62b935de942366 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -5,8 +5,7 @@ require('internal/modules/cjs/loader'); const { FunctionPrototypeBind, - ObjectSetPrototypeOf, - SafeMap, + ObjectSetPrototypeOf } = primordials; const { @@ -46,9 +45,6 @@ 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 bb3528eddde964..45abbae41193f4 100644 --- a/lib/internal/modules/esm/translators.js +++ b/lib/internal/modules/esm/translators.js @@ -11,6 +11,8 @@ const { StringPrototypeReplace, } = primordials; +const assert = require('internal/assert'); + let _TYPES = null; function lazyTypes() { if (_TYPES !== null) return _TYPES; @@ -132,27 +134,21 @@ 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); - }); - } + let filename = internalURLModule.fileURLToPath(new URL(url)); + if (isWindows) + filename = StringPrototypeReplace(filename, winSepRegEx, '\\'); 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); + 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); }); }); diff --git a/test/es-module/test-esm-snapshot.mjs b/test/es-module/test-esm-snapshot.mjs index e2695d20a81747..6482e3df605252 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, 1); +assert.strictEqual(one, 2);