Skip to content

Commit 1988563

Browse files
serchersercher
committed
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: nodejs/node#47194 Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent bf56be1 commit 1988563

File tree

8 files changed

+143
-93
lines changed

8 files changed

+143
-93
lines changed

‎graal-nodejs/lib/internal/bootstrap/realm.js‎

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,15 @@ const legacyWrapperList = new SafeSet([
128128
'util',
129129
]);
130130

131+
// The code bellow assumes that the two lists must not contain any modules
132+
// beginning with "internal/".
131133
// Modules that can only be imported via the node: scheme.
132134
const schemelessBlockList = new SafeSet([
133135
'test',
134136
'test/reporters',
135137
]);
138+
// Modules that will only be enabled at run time.
139+
const experimentalModuleList = new SafeSet();
136140

137141
// Set up process.binding() and process._linkedBinding().
138142
{
@@ -199,6 +203,20 @@ const getOwn = (target, property, receiver) => {
199203
undefined;
200204
};
201205

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

221238
// The CJS exports object of the module.
222239
this.exports = {};
@@ -238,14 +255,23 @@ class BuiltinModule {
238255
this.exportKeys = undefined;
239256
}
240257

258+
static allowRequireByUsers(id) {
259+
if (id === selfId) {
260+
// No code because this is an assertion against bugs.
261+
// eslint-disable-next-line no-restricted-syntax
262+
throw new Error(`Should not allow ${id}`);
263+
}
264+
canBeRequiredByUsersList.add(id);
265+
if (!schemelessBlockList.has(id)) {
266+
canBeRequiredByUsersWithoutSchemeList.add(id);
267+
}
268+
}
269+
241270
// To be called during pre-execution when --expose-internals is on.
242271
// Enables the user-land module loader to access internal modules.
243272
static exposeInternals() {
244-
for (const { 0: id, 1: mod } of BuiltinModule.map) {
245-
// Do not expose this to user land even with --expose-internals.
246-
if (id !== selfId) {
247-
mod.canBeRequiredByUsers = true;
248-
}
273+
for (let i = 0; i < internalBuiltinIds.length; ++i) {
274+
BuiltinModule.allowRequireByUsers(internalBuiltinIds[i]);
249275
}
250276
}
251277

@@ -254,14 +280,23 @@ class BuiltinModule {
254280
}
255281

256282
static canBeRequiredByUsers(id) {
257-
const mod = BuiltinModule.map.get(id);
258-
return mod && mod.canBeRequiredByUsers;
283+
return canBeRequiredByUsersList.has(id);
259284
}
260285

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

267302
static getSchemeOnlyModuleNames() {
@@ -270,7 +305,7 @@ class BuiltinModule {
270305

271306
// Used by user-land module loaders to compile and load builtins.
272307
compileForPublicLoader() {
273-
if (!this.canBeRequiredByUsers) {
308+
if (!BuiltinModule.canBeRequiredByUsers(this.id)) {
274309
// No code because this is an assertion against bugs
275310
// eslint-disable-next-line no-restricted-syntax
276311
throw new Error(`Should not compile ${this.id} for public use`);

‎graal-nodejs/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,
@@ -52,7 +51,6 @@ const {
5251
ReflectSet,
5352
RegExpPrototypeExec,
5453
SafeMap,
55-
SafeSet,
5654
SafeWeakMap,
5755
String,
5856
StringPrototypeCharAt,
@@ -82,7 +80,7 @@ const {
8280
} = require('internal/source_map/source_map_cache');
8381
const { pathToFileURL, fileURLToPath, isURL } = require('internal/url');
8482
const {
85-
deprecate,
83+
pendingDeprecate,
8684
emitExperimentalWarning,
8785
kEmptyObject,
8886
filterOwnProperties,
@@ -310,44 +308,29 @@ let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
310308
debug = fn;
311309
});
312310

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

352335
initializeCjsConditions();
353336

@@ -803,7 +786,6 @@ Module._resolveLookupPaths = function(request, parent) {
803786
StringPrototypeStartsWith(request, 'node:') &&
804787
BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5))
805788
) || (
806-
BuiltinModule.canBeRequiredByUsers(request) &&
807789
BuiltinModule.canBeRequiredWithoutScheme(request)
808790
)) {
809791
debug('looking for %j in []', request);
@@ -925,11 +907,11 @@ Module._load = function(request, parent, isMain) {
925907
// Slice 'node:' prefix
926908
const id = StringPrototypeSlice(request, 5);
927909

928-
const module = loadBuiltinModule(id, request);
929-
if (!module?.canBeRequiredByUsers) {
910+
if (!BuiltinModule.canBeRequiredByUsers(id)) {
930911
throw new ERR_UNKNOWN_BUILTIN_MODULE(request);
931912
}
932913

914+
const module = loadBuiltinModule(id, request);
933915
return module.exports;
934916
}
935917

@@ -947,9 +929,8 @@ Module._load = function(request, parent, isMain) {
947929
}
948930
}
949931

950-
const mod = loadBuiltinModule(filename, request);
951-
if (mod?.canBeRequiredByUsers &&
952-
BuiltinModule.canBeRequiredWithoutScheme(filename)) {
932+
if (BuiltinModule.canBeRequiredWithoutScheme(filename)) {
933+
const mod = loadBuiltinModule(filename, request);
953934
return mod.exports;
954935
}
955936

@@ -1003,7 +984,6 @@ Module._resolveFilename = function(request, parent, isMain, options) {
1003984
StringPrototypeStartsWith(request, 'node:') &&
1004985
BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5))
1005986
) || (
1006-
BuiltinModule.canBeRequiredByUsers(request) &&
1007987
BuiltinModule.canBeRequiredWithoutScheme(request)
1008988
)
1009989
) {
@@ -1459,8 +1439,7 @@ Module._preloadModules = function(requests) {
14591439

14601440
Module.syncBuiltinESMExports = function syncBuiltinESMExports() {
14611441
for (const mod of BuiltinModule.map.values()) {
1462-
if (mod.canBeRequiredByUsers &&
1463-
BuiltinModule.canBeRequiredWithoutScheme(mod.id)) {
1442+
if (BuiltinModule.canBeRequiredWithoutScheme(mod.id)) {
14641443
mod.syncExports();
14651444
}
14661445
}

‎graal-nodejs/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);

‎graal-nodejs/lib/internal/modules/esm/resolve.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -803,8 +803,7 @@ function parsePackageName(specifier, base) {
803803
* @returns {resolved: URL, format? : string}
804804
*/
805805
function packageResolve(specifier, base, conditions) {
806-
if (BuiltinModule.canBeRequiredByUsers(specifier) &&
807-
BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
806+
if (BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
808807
return new URL('node:' + specifier);
809808
}
810809

@@ -990,8 +989,7 @@ function checkIfDisallowedImport(specifier, parsed, parsedParentURL) {
990989

991990
return { url: parsed.href };
992991
}
993-
if (BuiltinModule.canBeRequiredByUsers(specifier) &&
994-
BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
992+
if (BuiltinModule.canBeRequiredWithoutScheme(specifier)) {
995993
throw new ERR_NETWORK_IMPORT_DISALLOWED(
996994
specifier,
997995
parsedParentURL,

‎graal-nodejs/lib/internal/modules/helpers.js‎

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

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

7172
let $Module = null;
@@ -99,8 +100,9 @@ function makeRequireFunction(mod, redirects) {
99100
const { href, protocol } = destination;
100101
if (protocol === 'node:') {
101102
const specifier = destination.pathname;
102-
const mod = loadBuiltinModule(specifier, href);
103-
if (mod && mod.canBeRequiredByUsers) {
103+
104+
if (BuiltinModule.canBeRequiredByUsers(specifier)) {
105+
const mod = loadBuiltinModule(specifier, href);
104106
return mod.exports;
105107
}
106108
throw new ERR_UNKNOWN_BUILTIN_MODULE(specifier);

‎graal-nodejs/lib/internal/process/warning.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ function emitWarning(warning, type, code, ctor) {
166166
process.nextTick(doEmitWarning, warning);
167167
}
168168

169-
function emitWarningSync(warning) {
170-
process.emit('warning', createWarningObject(warning));
169+
function emitWarningSync(warning, type, code, ctor) {
170+
process.emit('warning', createWarningObject(warning, type, code, ctor));
171171
}
172172

173173
function createWarningObject(warning, type, code, ctor, detail) {

0 commit comments

Comments
 (0)