From 631c3ef3de42685491bece4d1bdbd6feffc11b0e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Wed, 5 Apr 2023 13:13:23 +0200 Subject: [PATCH] module: do less CJS module loader initialization at run time This patch: - Builds the set of modules that can be required by users with/without the `node:` prefix at snapshot building time. We only modify it when `--expose-internals` but the default set is now in the snapshot. At run time the CJS module loader only creates a frozen array out of it. - `BuiltinModule.canBeRequiredWithoutScheme()` is now enough to determine if an id can be required without `node:` without an additional call to `BuiltinModule.canBeRequiredByUsers()` - Replace the pending-to-deprecate methods on `Module` with an internal implementation that only queries the CLI flags when being invoked. So we can install these methods in the snapshot. PR-URL: https://github.com/nodejs/node/pull/47194 Reviewed-By: Geoffrey Booth Reviewed-By: Chengzhong Wu --- lib/internal/bootstrap/realm.js | 60 ++++++++++++++++++----- lib/internal/modules/cjs/loader.js | 75 +++++++++++------------------ lib/internal/modules/esm/hooks.js | 3 +- lib/internal/modules/esm/resolve.js | 6 +-- lib/internal/modules/helpers.js | 18 ++++--- lib/internal/util.js | 64 ++++++++++++++++++------ 6 files changed, 137 insertions(+), 89 deletions(-) diff --git a/lib/internal/bootstrap/realm.js b/lib/internal/bootstrap/realm.js index 4b03d8247cb4e3..c8cdaeff7d369b 100644 --- a/lib/internal/bootstrap/realm.js +++ b/lib/internal/bootstrap/realm.js @@ -63,6 +63,7 @@ const { SafeSet, String, StringPrototypeStartsWith, + StringPrototypeSlice, TypeError, } = primordials; @@ -126,10 +127,14 @@ const legacyWrapperList = new SafeSet([ 'util', ]); +// The code bellow assumes that the two lists must not contain any modules +// beginning with "internal/". // Modules that can only be imported via the node: scheme. const schemelessBlockList = new SafeSet([ 'test', ]); +// Modules that will only be enabled at run time. +const experimentalModuleList = new SafeSet(); // Set up process.binding() and process._linkedBinding(). { @@ -196,6 +201,20 @@ const getOwn = (target, property, receiver) => { undefined; }; +const publicBuiltinIds = builtinIds + .filter((id) => + !StringPrototypeStartsWith(id, 'internal/') && + !experimentalModuleList.has(id), + ); +// Do not expose the loaders to user land even with --expose-internals. +const internalBuiltinIds = builtinIds + .filter((id) => StringPrototypeStartsWith(id, 'internal/') && id !== selfId); + +// When --expose-internals is on we'll add the internal builtin ids to these. +const canBeRequiredByUsersList = new SafeSet(publicBuiltinIds); +const canBeRequiredByUsersWithoutSchemeList = + new SafeSet(publicBuiltinIds.filter((id) => !schemelessBlockList.has(id))); + /** * An internal abstraction for the built-in JavaScript modules of Node.js. * Be careful not to expose this to user land unless --expose-internals is @@ -213,7 +232,6 @@ class BuiltinModule { constructor(id) { this.filename = `${id}.js`; this.id = id; - this.canBeRequiredByUsers = !StringPrototypeStartsWith(id, 'internal/'); // The CJS exports object of the module. this.exports = {}; @@ -235,14 +253,23 @@ class BuiltinModule { this.exportKeys = undefined; } + static allowRequireByUsers(id) { + if (id === selfId) { + // No code because this is an assertion against bugs. + // eslint-disable-next-line no-restricted-syntax + throw new Error(`Should not allow ${id}`); + } + canBeRequiredByUsersList.add(id); + if (!schemelessBlockList.has(id)) { + canBeRequiredByUsersWithoutSchemeList.add(id); + } + } + // To be called during pre-execution when --expose-internals is on. // Enables the user-land module loader to access internal modules. static exposeInternals() { - for (const { 0: id, 1: mod } of BuiltinModule.map) { - // Do not expose this to user land even with --expose-internals. - if (id !== selfId) { - mod.canBeRequiredByUsers = true; - } + for (let i = 0; i < internalBuiltinIds.length; ++i) { + BuiltinModule.allowRequireByUsers(internalBuiltinIds[i]); } } @@ -251,14 +278,23 @@ class BuiltinModule { } static canBeRequiredByUsers(id) { - const mod = BuiltinModule.map.get(id); - return mod && mod.canBeRequiredByUsers; + return canBeRequiredByUsersList.has(id); } - // Determine if a core module can be loaded without the node: prefix. This - // function does not validate if the module actually exists. static canBeRequiredWithoutScheme(id) { - return !schemelessBlockList.has(id); + return canBeRequiredByUsersWithoutSchemeList.has(id); + } + + static isBuiltin(id) { + return BuiltinModule.canBeRequiredWithoutScheme(id) || ( + typeof id === 'string' && + StringPrototypeStartsWith(id, 'node:') && + BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(id, 5)) + ); + } + + static getCanBeRequiredByUsersWithoutSchemeList() { + return ArrayFrom(canBeRequiredByUsersWithoutSchemeList); } static getSchemeOnlyModuleNames() { @@ -267,7 +303,7 @@ class BuiltinModule { // Used by user-land module loaders to compile and load builtins. compileForPublicLoader() { - if (!this.canBeRequiredByUsers) { + if (!BuiltinModule.canBeRequiredByUsers(this.id)) { // No code because this is an assertion against bugs // eslint-disable-next-line no-restricted-syntax throw new Error(`Should not compile ${this.id} for public use`); diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index af512195d21c62..3cab6d3ade3f93 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -34,7 +34,6 @@ const { ArrayPrototypeSplice, ArrayPrototypeUnshift, ArrayPrototypeUnshiftApply, - ArrayPrototypeFlatMap, Boolean, Error, JSONParse, @@ -51,7 +50,6 @@ const { ReflectSet, RegExpPrototypeExec, SafeMap, - SafeSet, SafeWeakMap, String, StringPrototypeCharAt, @@ -81,7 +79,7 @@ const { } = require('internal/source_map/source_map_cache'); const { pathToFileURL, fileURLToPath, isURL } = require('internal/url'); const { - deprecate, + pendingDeprecate, emitExperimentalWarning, kEmptyObject, filterOwnProperties, @@ -309,44 +307,29 @@ let debug = require('internal/util/debuglog').debuglog('module', (fn) => { debug = fn; }); -const builtinModules = []; +ObjectDefineProperty(Module.prototype, 'parent', { + __proto__: null, + get: pendingDeprecate( + getModuleParent, + 'module.parent is deprecated due to accuracy issues. Please use ' + + 'require.main to find program entry point instead.', + 'DEP0144', + ), + set: pendingDeprecate( + setModuleParent, + 'module.parent is deprecated due to accuracy issues. Please use ' + + 'require.main to find program entry point instead.', + 'DEP0144', + ), +}); +Module._debug = pendingDeprecate(debug, 'Module._debug is deprecated.', 'DEP0077'); +Module.isBuiltin = BuiltinModule.isBuiltin; + // This function is called during pre-execution, before any user code is run. function initializeCJS() { - const pendingDeprecation = getOptionValue('--pending-deprecation'); - ObjectDefineProperty(Module.prototype, 'parent', { - __proto__: null, - get: pendingDeprecation ? deprecate( - getModuleParent, - 'module.parent is deprecated due to accuracy issues. Please use ' + - 'require.main to find program entry point instead.', - 'DEP0144', - ) : getModuleParent, - set: pendingDeprecation ? deprecate( - setModuleParent, - 'module.parent is deprecated due to accuracy issues. Please use ' + - 'require.main to find program entry point instead.', - 'DEP0144', - ) : setModuleParent, - }); - Module._debug = deprecate(debug, 'Module._debug is deprecated.', 'DEP0077'); - - for (const { 0: id, 1: mod } of BuiltinModule.map) { - if (mod.canBeRequiredByUsers && - BuiltinModule.canBeRequiredWithoutScheme(id)) { - ArrayPrototypePush(builtinModules, id); - } - } - - const allBuiltins = new SafeSet( - ArrayPrototypeFlatMap(builtinModules, (bm) => [bm, `node:${bm}`]), - ); - BuiltinModule.getSchemeOnlyModuleNames().forEach((builtin) => allBuiltins.add(`node:${builtin}`)); - ObjectFreeze(builtinModules); - Module.builtinModules = builtinModules; - - Module.isBuiltin = function isBuiltin(moduleName) { - return allBuiltins.has(moduleName); - }; + // This need to be done at runtime in case --expose-internals is set. + const builtinModules = BuiltinModule.getCanBeRequiredByUsersWithoutSchemeList(); + Module.builtinModules = ObjectFreeze(builtinModules); initializeCjsConditions(); @@ -813,7 +796,6 @@ Module._resolveLookupPaths = function(request, parent) { StringPrototypeStartsWith(request, 'node:') && BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5)) ) || ( - BuiltinModule.canBeRequiredByUsers(request) && BuiltinModule.canBeRequiredWithoutScheme(request) )) { debug('looking for %j in []', request); @@ -935,11 +917,11 @@ Module._load = function(request, parent, isMain) { // Slice 'node:' prefix const id = StringPrototypeSlice(request, 5); - const module = loadBuiltinModule(id, request); - if (!module?.canBeRequiredByUsers) { + if (!BuiltinModule.canBeRequiredByUsers(id)) { throw new ERR_UNKNOWN_BUILTIN_MODULE(request); } + const module = loadBuiltinModule(id, request); return module.exports; } @@ -957,9 +939,8 @@ Module._load = function(request, parent, isMain) { } } - const mod = loadBuiltinModule(filename, request); - if (mod?.canBeRequiredByUsers && - BuiltinModule.canBeRequiredWithoutScheme(filename)) { + if (BuiltinModule.canBeRequiredWithoutScheme(filename)) { + const mod = loadBuiltinModule(filename, request); return mod.exports; } @@ -1013,7 +994,6 @@ Module._resolveFilename = function(request, parent, isMain, options) { StringPrototypeStartsWith(request, 'node:') && BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5)) ) || ( - BuiltinModule.canBeRequiredByUsers(request) && BuiltinModule.canBeRequiredWithoutScheme(request) ) ) { @@ -1469,8 +1449,7 @@ Module._preloadModules = function(requests) { Module.syncBuiltinESMExports = function syncBuiltinESMExports() { for (const mod of BuiltinModule.map.values()) { - if (mod.canBeRequiredByUsers && - BuiltinModule.canBeRequiredWithoutScheme(mod.id)) { + if (BuiltinModule.canBeRequiredWithoutScheme(mod.id)) { mod.syncExports(); } } diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 0597aaa79d9ef1..513204bb9d7e48 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -207,8 +207,7 @@ class Hooks { globalThis, // Param getBuiltin (builtinName) => { - if (BuiltinModule.canBeRequiredByUsers(builtinName) && - BuiltinModule.canBeRequiredWithoutScheme(builtinName)) { + if (BuiltinModule.canBeRequiredWithoutScheme(builtinName)) { return require(builtinName); } throw new ERR_INVALID_ARG_VALUE('builtinName', builtinName); diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 58373230da3587..5212e2ce17758d 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -736,8 +736,7 @@ function parsePackageName(specifier, base) { * @returns {resolved: URL, format? : string} */ function packageResolve(specifier, base, conditions) { - if (BuiltinModule.canBeRequiredByUsers(specifier) && - BuiltinModule.canBeRequiredWithoutScheme(specifier)) { + if (BuiltinModule.canBeRequiredWithoutScheme(specifier)) { return new URL('node:' + specifier); } @@ -919,8 +918,7 @@ function checkIfDisallowedImport(specifier, parsed, parsedParentURL) { return { url: parsed.href }; } - if (BuiltinModule.canBeRequiredByUsers(specifier) && - BuiltinModule.canBeRequiredWithoutScheme(specifier)) { + if (BuiltinModule.canBeRequiredWithoutScheme(specifier)) { throw new ERR_NETWORK_IMPORT_DISALLOWED( specifier, parsedParentURL, diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index d20ef5e90f3f39..eb227e205c260f 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -58,13 +58,14 @@ function getCjsConditions() { } function loadBuiltinModule(filename, request) { - const mod = BuiltinModule.map.get(filename); - if (mod?.canBeRequiredByUsers) { - debug('load built-in module %s', request); - // compileForPublicLoader() throws if mod.canBeRequiredByUsers is false: - mod.compileForPublicLoader(); - return mod; + if (!BuiltinModule.canBeRequiredByUsers(filename)) { + return; } + const mod = BuiltinModule.map.get(filename); + debug('load built-in module %s', request); + // compileForPublicLoader() throws if canBeRequiredByUsers is false: + mod.compileForPublicLoader(); + return mod; } // Invoke with makeRequireFunction(module) where |module| is the Module object @@ -88,8 +89,9 @@ function makeRequireFunction(mod, redirects) { const href = destination.href; if (destination.protocol === 'node:') { const specifier = destination.pathname; - const mod = loadBuiltinModule(specifier, href); - if (mod && mod.canBeRequiredByUsers) { + + if (BuiltinModule.canBeRequiredByUsers(specifier)) { + const mod = loadBuiltinModule(specifier, href); return mod.exports; } throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier); diff --git a/lib/internal/util.js b/lib/internal/util.js index 1c8a02bd1811d8..2ec432f838087b 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -62,6 +62,7 @@ const { toUSVString: _toUSVString, } = internalBinding('util'); const { isNativeError } = internalBinding('types'); +const { getOptionValue } = require('internal/options'); const noCrypto = !process.versions.openssl; @@ -105,6 +106,48 @@ const codesWarned = new SafeSet(); let validateString; +function getDeprecationWarningEmitter( + code, msg, deprecated, useEmitSync, + shouldEmitWarning = () => true, +) { + let warned = false; + return function() { + if (!warned && shouldEmitWarning()) { + warned = true; + if (code !== undefined) { + if (!codesWarned.has(code)) { + const emitWarning = useEmitSync ? + require('internal/process/warning').emitWarningSync : + process.emitWarning; + emitWarning(msg, 'DeprecationWarning', code, deprecated); + codesWarned.add(code); + } + } else { + process.emitWarning(msg, 'DeprecationWarning', deprecated); + } + } + }; +} + +function isPendingDeprecation() { + return getOptionValue('--pending-deprecation') && + !getOptionValue('--no-deprecation'); +} + +// Internal deprecator for pending --pending-deprecation. This can be invoked +// at snapshot building time as the warning permission is only queried at +// run time. +function pendingDeprecate(fn, msg, code) { + const emitDeprecationWarning = getDeprecationWarningEmitter( + code, msg, deprecated, false, isPendingDeprecation, + ); + function deprecated(...args) { + emitDeprecationWarning(); + return ReflectApply(fn, this, args); + } + return deprecated; +} + // Mark that a method should not be used. // Returns a modified function which warns once by default. // If --no-deprecation is set, then it is a no-op. @@ -120,22 +163,12 @@ function deprecate(fn, msg, code, useEmitSync) { if (code !== undefined) validateString(code, 'code'); - let warned = false; + const emitDeprecationWarning = getDeprecationWarningEmitter( + code, msg, deprecated, useEmitSync, + ); + function deprecated(...args) { - if (!warned) { - warned = true; - if (code !== undefined) { - if (!codesWarned.has(code)) { - const emitWarning = useEmitSync ? - require('internal/process/warning').emitWarningSync : - process.emitWarning; - emitWarning(msg, 'DeprecationWarning', code, deprecated); - codesWarned.add(code); - } - } else { - process.emitWarning(msg, 'DeprecationWarning', deprecated); - } - } + emitDeprecationWarning(); if (new.target) { return ReflectConstruct(fn, args, new.target); } @@ -812,4 +845,5 @@ module.exports = { kEmptyObject, kEnumerableProperty, setOwnProperty, + pendingDeprecate, };