Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: use cjsCache over esm injection #34605

Closed
wants to merge 3 commits into from
Closed

module: use cjsCache over esm injection #34605

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76d85fc
module: use cjsCache over esm injection
guybedford Aug 3, 2020
7f9861b
weakmap approach
guybedford Aug 3, 2020
2e0ec3f
Update lib/internal/modules/cjs/loader.js
guybedford Aug 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 6 additions & 23 deletions lib/internal/modules/cjs/loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1120,29 +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;
// 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 ||
guybedford marked this conversation as resolved.
Show resolved Hide resolved
module.module.getStatus() < kEvaluated) &&
!ESMLoader.cjsCache.has(this))
ESMLoader.cjsCache.set(this, exports);
};


Expand Down
4 changes: 2 additions & 2 deletions lib/internal/modules/esm/loader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require('internal/modules/cjs/loader');
const {
FunctionPrototypeBind,
ObjectSetPrototypeOf,
SafeMap,
SafeWeakMap,
} = primordials;

const {
Expand Down Expand Up @@ -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.
Expand Down
42 changes: 14 additions & 28 deletions lib/internal/modules/esm/translators.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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);
});
});

Expand Down