module: do less CJS module loader initialization at run time

joyeecheung · RafaelGSS · commit 0b661ec35a9b · 2023-04-06T11:24:01.000-03:00
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 <webadmin@geoffreybooth.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
diff --git 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
@@ -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, isURLInstance } = require('internal/url');
 const {
-  deprecate,
+  pendingDeprecate,
   emitExperimentalWarning,
   kEmptyObject,
   filterOwnProperties,
@@ -308,44 +306,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();
 
@@ -801,7 +784,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);
@@ -923,11 +905,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;
   }
 
@@ -945,9 +927,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;
   }
 
@@ -1001,7 +982,6 @@ Module._resolveFilename = function(request, parent, isMain, options) {
       StringPrototypeStartsWith(request, 'node:') &&
       BuiltinModule.canBeRequiredByUsers(StringPrototypeSlice(request, 5))
     ) || (
-      BuiltinModule.canBeRequiredByUsers(request) &&
       BuiltinModule.canBeRequiredWithoutScheme(request)
     )
   ) {
@@ -1457,8 +1437,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
@@ -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
@@ -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
@@ -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
