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.

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: do less CJS module loader initialization at run time #47194

Merged
merged 1 commit into from
Apr 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 48 additions & 12 deletions lib/internal/bootstrap/realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const {
SafeSet,
String,
StringPrototypeStartsWith,
StringPrototypeSlice,
TypeError,
} = primordials;

Expand Down Expand Up @@ -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().
{
Expand Down Expand Up @@ -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
Expand All @@ -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 = {};
Expand All @@ -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]);
}
}

Expand All @@ -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() {
Expand All @@ -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`);
Expand Down
75 changes: 27 additions & 48 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const {
ArrayPrototypeSplice,
ArrayPrototypeUnshift,
ArrayPrototypeUnshiftApply,
ArrayPrototypeFlatMap,
Boolean,
Error,
JSONParse,
Expand All @@ -51,7 +50,6 @@ const {
ReflectSet,
RegExpPrototypeExec,
SafeMap,
SafeSet,
SafeWeakMap,
String,
StringPrototypeCharAt,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)) {
joyeecheung marked this conversation as resolved.
Show resolved Hide resolved
throw new ERR_UNKNOWN_BUILTIN_MODULE(request);
}

const module = loadBuiltinModule(id, request);
return module.exports;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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)
)
) {
Expand Down Expand Up @@ -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();
}
}
Expand Down
3 changes: 1 addition & 2 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down
18 changes: 10 additions & 8 deletions lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
Loading