Skip to content

Commit 631c3ef

Browse files
joyeecheungRafaelGSS
authored andcommitted
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: #47194 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent fb047d6 commit 631c3ef

File tree

6 files changed

+137
-89
lines changed

6 files changed

+137
-89
lines changed

lib/internal/bootstrap/realm.js

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ const {
6363
SafeSet,
6464
String,
6565
StringPrototypeStartsWith,
66+
StringPrototypeSlice,
6667
TypeError,
6768
} = primordials;
6869

@@ -126,10 +127,14 @@ const legacyWrapperList = new SafeSet([
126127
'util',
127128
]);
128129

130+
// The code bellow assumes that the two lists must not contain any modules
131+
// beginning with "internal/".
129132
// Modules that can only be imported via the node: scheme.
130133
const schemelessBlockList = new SafeSet([
131134
'test',
132135
]);
136+
// Modules that will only be enabled at run time.
137+
const experimentalModuleList = new SafeSet();
133138

134139
// Set up process.binding() and process._linkedBinding().
135140
{
@@ -196,6 +201,20 @@ const getOwn = (target, property, receiver) => {
196201
undefined;
197202
};
198203

204+
const publicBuiltinIds = builtinIds
205+
.filter((id) =>
206+
!StringPrototypeStartsWith(id, 'internal/') &&
207+
!experimentalModuleList.has(id),
208+
);
209+
// Do not expose the loaders to user land even with --expose-internals.
210+
const internalBuiltinIds = builtinIds
211+
.filter((id) => StringPrototypeStartsWith(id, 'internal/') && id !== selfId);
212+
213+
// When --expose-internals is on we'll add the internal builtin ids to these.
214+
const canBeRequiredByUsersList = new SafeSet(publicBuiltinIds);
215+
const canBeRequiredByUsersWithoutSchemeList =
216+
new SafeSet(publicBuiltinIds.filter((id) => !schemelessBlockList.has(id)));
217+
199218
/**
200219
* An internal abstraction for the built-in JavaScript modules of Node.js.
201220
* Be careful not to expose this to user land unless --expose-internals is
@@ -213,7 +232,6 @@ class BuiltinModule {
213232
constructor(id) {
214233
this.filename = `${id}.js`;
215234
this.id = id;
216-
this.canBeRequiredByUsers = !StringPrototypeStartsWith(id, 'internal/');
217235

218236
// The CJS exports object of the module.
219237
this.exports = {};
@@ -235,14 +253,23 @@ class BuiltinModule {
235253
this.exportKeys = undefined;
236254
}
237255

256+
static allowRequireByUsers(id) {
257+
if (id === selfId) {
258+
// No code because this is an assertion against bugs.
259+
// eslint-disable-next-line no-restricted-syntax
260+
throw new Error(`Should not allow ${id}`);
261+
}
262+
canBeRequiredByUsersList.add(id);
263+
if (!schemelessBlockList.has(id)) {
264+
canBeRequiredByUsersWithoutSchemeList.add(id);
265+
}
266+
}
267+
238268
// To be called during pre-execution when --expose-internals is on.
239269
// Enables the user-land module loader to access internal modules.
240270
static exposeInternals() {
241-
for (const { 0: id, 1: mod } of BuiltinModule.map) {
242-
// Do not expose this to user land even with --expose-internals.
243-
if (id !== selfId) {
244-
mod.canBeRequiredByUsers = true;
245-
}
271+
for (let i = 0; i < internalBuiltinIds.length; ++i) {
272+
BuiltinModule.allowRequireByUsers(internalBuiltinIds[i]);
246273
}
247274
}
248275

@@ -251,14 +278,23 @@ class BuiltinModule {
251278
}
252279

253280
static canBeRequiredByUsers(id) {
254-
const mod = BuiltinModule.map.get(id);
255-
return mod && mod.canBeRequiredByUsers;
281+
return canBeRequiredByUsersList.has(id);
256282
}
257283

258-
// Determine if a core module can be loaded without the node: prefix. This
259-
// function does not validate if the module actually exists.
260284
static canBeRequiredWithoutScheme(id) {
261-
return !schemelessBlockList.has(id);
285+
return canBeRequiredByUsersWithoutSchemeList.has(id);
286+
}
287+
288+
static isBuiltin(id) {
289+
return BuiltinModule.canBeRequiredWithoutScheme(id) || (
290+
typeof id === 'string' &&
291+
StringPrototypeStartsWith(id, 'node:') &&
292+
BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(id, 5))
293+
);
294+
}
295+
296+
static getCanBeRequiredByUsersWithoutSchemeList() {
297+
return ArrayFrom(canBeRequiredByUsersWithoutSchemeList);
262298
}
263299

264300
static getSchemeOnlyModuleNames() {
@@ -267,7 +303,7 @@ class BuiltinModule {
267303

268304
// Used by user-land module loaders to compile and load builtins.
269305
compileForPublicLoader() {
270-
if (!this.canBeRequiredByUsers) {
306+
if (!BuiltinModule.canBeRequiredByUsers(this.id)) {
271307
// No code because this is an assertion against bugs
272308
// eslint-disable-next-line no-restricted-syntax
273309
throw new Error(`Should not compile ${this.id} for public use`);

lib/internal/modules/cjs/loader.js

Lines changed: 27 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ const {
3434
ArrayPrototypeSplice,
3535
ArrayPrototypeUnshift,
3636
ArrayPrototypeUnshiftApply,
37-
ArrayPrototypeFlatMap,
3837
Boolean,
3938
Error,
4039
JSONParse,
@@ -51,7 +50,6 @@ const {
5150
ReflectSet,
5251
RegExpPrototypeExec,
5352
SafeMap,
54-
SafeSet,
5553
SafeWeakMap,
5654
String,
5755
StringPrototypeCharAt,
@@ -81,7 +79,7 @@ const {
8179
} = require('internal/source_map/source_map_cache');
8280
const { pathToFileURL, fileURLToPath, isURL } = require('internal/url');
8381
const {
84-
deprecate,
82+
pendingDeprecate,
8583
emitExperimentalWarning,
8684
kEmptyObject,
8785
filterOwnProperties,
@@ -309,44 +307,29 @@ let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
309307
debug = fn;
310308
});
311309

312-
const builtinModules = [];
310+
ObjectDefineProperty(Module.prototype, 'parent', {
311+
__proto__: null,
312+
get: pendingDeprecate(
313+
getModuleParent,
314+
'module.parent is deprecated due to accuracy issues. Please use ' +
315+
'require.main to find program entry point instead.',
316+
'DEP0144',
317+
),
318+
set: pendingDeprecate(
319+
setModuleParent,
320+
'module.parent is deprecated due to accuracy issues. Please use ' +
321+
'require.main to find program entry point instead.',
322+
'DEP0144',
323+
),
324+
});
325+
Module._debug = pendingDeprecate(debug, 'Module._debug is deprecated.', 'DEP0077');
326+
Module.isBuiltin = BuiltinModule.isBuiltin;
327+
313328
// This function is called during pre-execution, before any user code is run.
314329
function initializeCJS() {
315-
const pendingDeprecation = getOptionValue('--pending-deprecation');
316-
ObjectDefineProperty(Module.prototype, 'parent', {
317-
__proto__: null,
318-
get: pendingDeprecation ? deprecate(
319-
getModuleParent,
320-
'module.parent is deprecated due to accuracy issues. Please use ' +
321-
'require.main to find program entry point instead.',
322-
'DEP0144',
323-
) : getModuleParent,
324-
set: pendingDeprecation ? deprecate(
325-
setModuleParent,
326-
'module.parent is deprecated due to accuracy issues. Please use ' +
327-
'require.main to find program entry point instead.',
328-
'DEP0144',
329-
) : setModuleParent,
330-
});
331-
Module._debug = deprecate(debug, 'Module._debug is deprecated.', 'DEP0077');
332-
333-
for (const { 0: id, 1: mod } of BuiltinModule.map) {
334-
if (mod.canBeRequiredByUsers &&
335-
BuiltinModule.canBeRequiredWithoutScheme(id)) {
336-
ArrayPrototypePush(builtinModules, id);
337-
}
338-
}
339-
340-
const allBuiltins = new SafeSet(
341-
ArrayPrototypeFlatMap(builtinModules, (bm) => [bm, `node:${bm}`]),
342-
);
343-
BuiltinModule.getSchemeOnlyModuleNames().forEach((builtin) => allBuiltins.add(`node:${builtin}`));
344-
ObjectFreeze(builtinModules);
345-
Module.builtinModules = builtinModules;
346-
347-
Module.isBuiltin = function isBuiltin(moduleName) {
348-
return allBuiltins.has(moduleName);
349-
};
330+
// This need to be done at runtime in case --expose-internals is set.
331+
const builtinModules = BuiltinModule.getCanBeRequiredByUsersWithoutSchemeList();
332+
Module.builtinModules = ObjectFreeze(builtinModules);
350333

351334
initializeCjsConditions();
352335

@@ -813,7 +796,6 @@ Module._resolveLookupPaths = function(request, parent) {
813796
StringPrototypeStartsWith(request, 'node:') &&
814797
BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5))
815798
) || (
816-
BuiltinModule.canBeRequiredByUsers(request) &&
817799
BuiltinModule.canBeRequiredWithoutScheme(request)
818800
)) {
819801
debug('looking for %j in []', request);
@@ -935,11 +917,11 @@ Module._load = function(request, parent, isMain) {
935917
// Slice 'node:' prefix
936918
const id = StringPrototypeSlice(request, 5);
937919

938-
const module = loadBuiltinModule(id, request);
939-
if (!module?.canBeRequiredByUsers) {
920+
if (!BuiltinModule.canBeRequiredByUsers(id)) {
940921
throw new ERR_UNKNOWN_BUILTIN_MODULE(request);
941922
}
942923

924+
const module = loadBuiltinModule(id, request);
943925
return module.exports;
944926
}
945927

@@ -957,9 +939,8 @@ Module._load = function(request, parent, isMain) {
957939
}
958940
}
959941

960-
const mod = loadBuiltinModule(filename, request);
961-
if (mod?.canBeRequiredByUsers &&
962-
BuiltinModule.canBeRequiredWithoutScheme(filename)) {
942+
if (BuiltinModule.canBeRequiredWithoutScheme(filename)) {
943+
const mod = loadBuiltinModule(filename, request);
963944
return mod.exports;
964945
}
965946

@@ -1013,7 +994,6 @@ Module._resolveFilename = function(request, parent, isMain, options) {
1013994
StringPrototypeStartsWith(request, 'node:') &&
1014995
BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5))
1015996
) || (
1016-
BuiltinModule.canBeRequiredByUsers(request) &&
1017997
BuiltinModule.canBeRequiredWithoutScheme(request)
1018998
)
1019999
) {
@@ -1469,8 +1449,7 @@ Module._preloadModules = function(requests) {
14691449

14701450
Module.syncBuiltinESMExports = function syncBuiltinESMExports() {
14711451
for (const mod of BuiltinModule.map.values()) {
1472-
if (mod.canBeRequiredByUsers &&
1473-
BuiltinModule.canBeRequiredWithoutScheme(mod.id)) {
1452+
if (BuiltinModule.canBeRequiredWithoutScheme(mod.id)) {
14741453
mod.syncExports();
14751454
}
14761455
}

lib/internal/modules/esm/hooks.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,7 @@ class Hooks {
207207
globalThis,
208208
// Param getBuiltin
209209
(builtinName) => {
210-
if (BuiltinModule.canBeRequiredByUsers(builtinName) &&
211-
BuiltinModule.canBeRequiredWithoutScheme(builtinName)) {
210+
if (BuiltinModule.canBeRequiredWithoutScheme(builtinName)) {
212211
return require(builtinName);
213212
}
214213
throw new ERR_INVALID_ARG_VALUE('builtinName', builtinName);

lib/internal/modules/esm/resolve.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -736,8 +736,7 @@ function parsePackageName(specifier, base) {
736736
* @returns {resolved: URL, format? : string}
737737
*/
738738
function packageResolve(specifier, base, conditions) {
739-
if (BuiltinModule.canBeRequiredByUsers(specifier) &&
740-
BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
739+
if (BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
741740
return new URL('node:' + specifier);
742741
}
743742

@@ -919,8 +918,7 @@ function checkIfDisallowedImport(specifier, parsed, parsedParentURL) {
919918

920919
return { url: parsed.href };
921920
}
922-
if (BuiltinModule.canBeRequiredByUsers(specifier) &&
923-
BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
921+
if (BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
924922
throw new ERR_NETWORK_IMPORT_DISALLOWED(
925923
specifier,
926924
parsedParentURL,

lib/internal/modules/helpers.js

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,14 @@ function getCjsConditions() {
5858
}
5959

6060
function loadBuiltinModule(filename, request) {
61-
const mod = BuiltinModule.map.get(filename);
62-
if (mod?.canBeRequiredByUsers) {
63-
debug('load built-in module %s', request);
64-
// compileForPublicLoader() throws if mod.canBeRequiredByUsers is false:
65-
mod.compileForPublicLoader();
66-
return mod;
61+
if (!BuiltinModule.canBeRequiredByUsers(filename)) {
62+
return;
6763
}
64+
const mod = BuiltinModule.map.get(filename);
65+
debug('load built-in module %s', request);
66+
// compileForPublicLoader() throws if canBeRequiredByUsers is false:
67+
mod.compileForPublicLoader();
68+
return mod;
6869
}
6970

7071
// Invoke with makeRequireFunction(module) where |module| is the Module object
@@ -88,8 +89,9 @@ function makeRequireFunction(mod, redirects) {
8889
const href = destination.href;
8990
if (destination.protocol === 'node:') {
9091
const specifier = destination.pathname;
91-
const mod = loadBuiltinModule(specifier, href);
92-
if (mod && mod.canBeRequiredByUsers) {
92+
93+
if (BuiltinModule.canBeRequiredByUsers(specifier)) {
94+
const mod = loadBuiltinModule(specifier, href);
9395
return mod.exports;
9496
}
9597
throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier);

0 commit comments

Comments
 (0)