module: do not share the internal require function with public loaders

This patch removes `NativeModule.require` and
`NativeModule.requireWithFallbackInDeps`. The public loaders now
have to use a special method
`NativeModule.prototype.compileForPublicLoader()` to compile native
modules. In addition this patch moves the decisions of proxifying
exports and throwing unknown builtin errors entirely to public
loaders, and skip those during internal use - therefore `loaders.js`,
which is compiled during bootstrap, no longer needs to be aware of
the value of `--experimental-modules`.

PR-URL: #26549
Reviewed-By: James M Snell <[email protected]>

Author: joyeecheung

Diff for: lib/internal/bootstrap/loaders.js

@@ -19,7 +19,7 @@
 //   can be created using NODE_MODULE_CONTEXT_AWARE_CPP() with the flag
 //   NM_F_LINKED.
 // - internalBinding(): the private internal C++ binding loader, inaccessible
-//   from user land because they are only available from NativeModule.require().
+//   from user land unless through `require('internal/test/binding')`.
 //   These C++ bindings are created using NODE_MODULE_CONTEXT_AWARE_INTERNAL()
 //   and have their nm_flags set to NM_F_INTERNAL.
 //
@@ -42,7 +42,7 @@
 // This file is compiled as if it's wrapped in a function with arguments
 // passed by node::RunBootstrapping()
 /* global process, getLinkedBinding, getInternalBinding */
-/* global experimentalModules, exposeInternals, primordials */
+/* global exposeInternals, primordials */
 
 const {
   Reflect,
@@ -185,27 +185,9 @@ function nativeModuleRequire(id) {
   }
 
   const mod = NativeModule.map.get(id);
-  if (!mod) {
-    // Model the error off the internal/errors.js model, but
-    // do not use that module given that it could actually be
-    // the one causing the error if there's a bug in Node.js.
-    // eslint-disable-next-line no-restricted-syntax
-    const err = new Error(`No such built-in module: ${id}`);
-    err.code = 'ERR_UNKNOWN_BUILTIN_MODULE';
-    err.name = 'Error [ERR_UNKNOWN_BUILTIN_MODULE]';
-    throw err;
-  }
-
-  if (mod.loaded || mod.loading) {
-    return mod.exports;
-  }
-
-  moduleLoadList.push(`NativeModule ${id}`);
-  mod.compile();
-  return mod.exports;
+  return mod.compile();
 }
 
-NativeModule.require = nativeModuleRequire;
 NativeModule.exists = function(id) {
   return NativeModule.map.has(id);
 };
@@ -217,11 +199,25 @@ NativeModule.canBeRequiredByUsers = function(id) {
 
 // Allow internal modules from dependencies to require
 // other modules from dependencies by providing fallbacks.
-NativeModule.requireWithFallbackInDeps = function(request) {
+function requireWithFallbackInDeps(request) {
   if (!NativeModule.map.has(request)) {
     request = `internal/deps/${request}`;
   }
-  return NativeModule.require(request);
+  return nativeModuleRequire(request);
+}
+
+// This is exposed for public loaders
+NativeModule.prototype.compileForPublicLoader = function(needToProxify) {
+  if (!this.canBeRequiredByUsers) {
+    // 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`);
+  }
+  this.compile();
+  if (needToProxify && !this.exportKeys) {
+    this.proxifyExports();
+  }
+  return this.exports;
 };
 
 const getOwn = (target, property, receiver) => {
@@ -289,26 +285,27 @@ NativeModule.prototype.proxifyExports = function() {
 };
 
 NativeModule.prototype.compile = function() {
-  const id = this.id;
+  if (this.loaded || this.loading) {
+    return this.exports;
+  }
 
+  const id = this.id;
   this.loading = true;
 
   try {
     const requireFn = this.id.startsWith('internal/deps/') ?
-      NativeModule.requireWithFallbackInDeps :
-      NativeModule.require;
+      requireWithFallbackInDeps : nativeModuleRequire;
 
     const fn = compileFunction(id);
     fn(this.exports, requireFn, this, process, internalBinding, primordials);
 
-    if (experimentalModules && this.canBeRequiredByUsers) {
-      this.proxifyExports();
-    }
-
     this.loaded = true;
   } finally {
     this.loading = false;
   }
+
+  moduleLoadList.push(`NativeModule ${id}`);
+  return this.exports;
 };
 
 // This will be passed to internal/bootstrap/node.js.

Diff for: lib/internal/bootstrap/node.js

@@ -33,13 +33,12 @@
 
 // This file is compiled as if it's wrapped in a function with arguments
 // passed by node::RunBootstrapping()
-/* global process, loaderExports, isMainThread, ownsProcessState */
+/* global process, require, internalBinding, isMainThread, ownsProcessState */
 /* global primordials */
 
-const { internalBinding, NativeModule } = loaderExports;
 const { Object, Symbol } = primordials;
 const config = internalBinding('config');
-const { deprecate } = NativeModule.require('internal/util');
+const { deprecate } = require('internal/util');
 
 setupProcessObject();
 
@@ -50,17 +49,17 @@ process.domain = null;
 process._exiting = false;
 
 // Bootstrappers for all threads, including worker threads and main thread
-const perThreadSetup = NativeModule.require('internal/process/per_thread');
+const perThreadSetup = require('internal/process/per_thread');
 // Bootstrappers for the main thread only
 let mainThreadSetup;
 // Bootstrappers for the worker threads only
 let workerThreadSetup;
 if (ownsProcessState) {
-  mainThreadSetup = NativeModule.require(
+  mainThreadSetup = require(
     'internal/process/main_thread_only'
   );
 } else {
-  workerThreadSetup = NativeModule.require(
+  workerThreadSetup = require(
     'internal/process/worker_thread_only'
   );
 }
@@ -116,14 +115,14 @@ if (isMainThread) {
 
 const {
   emitWarning
-} = NativeModule.require('internal/process/warning');
+} = require('internal/process/warning');
 
 process.emitWarning = emitWarning;
 
 const {
   nextTick,
   runNextTicks
-} = NativeModule.require('internal/process/next_tick').setup();
+} = require('internal/process/next_tick').setup();
 
 process.nextTick = nextTick;
 // Used to emulate a tick manually in the JS land.
@@ -161,7 +160,7 @@ if (credentials.implementsPosixCredentials) {
 
 if (isMainThread) {
   const { getStdout, getStdin, getStderr } =
-      NativeModule.require('internal/process/stdio').getMainThreadStdio();
+    require('internal/process/stdio').getMainThreadStdio();
   setupProcessStdio(getStdout, getStdin, getStderr);
 } else {
   const { getStdout, getStdin, getStderr } =
@@ -173,7 +172,7 @@ if (isMainThread) {
 // process. They use the same functions as the JS embedder API. These callbacks
 // are setup immediately to prevent async_wrap.setupHooks() from being hijacked
 // and the cost of doing so is negligible.
-const { nativeHooks } = NativeModule.require('internal/async_hooks');
+const { nativeHooks } = require('internal/async_hooks');
 internalBinding('async_wrap').setupHooks(nativeHooks);
 
 // XXX(joyeecheung): this has to be done after the initial load of
@@ -183,7 +182,7 @@ if (config.hasInspector) {
   const {
     enable,
     disable
-  } = NativeModule.require('internal/inspector_async_hook');
+  } = require('internal/inspector_async_hook');
   internalBinding('inspector').registerAsyncHook(enable, disable);
 }
 
@@ -193,22 +192,22 @@ if (!config.noBrowserGlobals) {
   // https://console.spec.whatwg.org/#console-namespace
   exposeNamespace(global, 'console', createGlobalConsole(global.console));
 
-  const { URL, URLSearchParams } = NativeModule.require('internal/url');
+  const { URL, URLSearchParams } = require('internal/url');
   // https://url.spec.whatwg.org/#url
   exposeInterface(global, 'URL', URL);
   // https://url.spec.whatwg.org/#urlsearchparams
   exposeInterface(global, 'URLSearchParams', URLSearchParams);
 
   const {
     TextEncoder, TextDecoder
-  } = NativeModule.require('internal/encoding');
+  } = require('internal/encoding');
   // https://encoding.spec.whatwg.org/#textencoder
   exposeInterface(global, 'TextEncoder', TextEncoder);
   // https://encoding.spec.whatwg.org/#textdecoder
   exposeInterface(global, 'TextDecoder', TextDecoder);
 
   // https://html.spec.whatwg.org/multipage/webappapis.html#windoworworkerglobalscope
-  const timers = NativeModule.require('timers');
+  const timers = require('timers');
   defineOperation(global, 'clearInterval', timers.clearInterval);
   defineOperation(global, 'clearTimeout', timers.clearTimeout);
   defineOperation(global, 'setInterval', timers.setInterval);
@@ -302,7 +301,7 @@ Object.defineProperty(process, 'features', {
     fatalException,
     setUncaughtExceptionCaptureCallback,
     hasUncaughtExceptionCaptureCallback
-  } = NativeModule.require('internal/process/execution');
+  } = require('internal/process/execution');
 
   process._fatalException = fatalException;
   process.setUncaughtExceptionCaptureCallback =
@@ -312,7 +311,7 @@ Object.defineProperty(process, 'features', {
 }
 
 function setupProcessObject() {
-  const EventEmitter = NativeModule.require('events');
+  const EventEmitter = require('events');
   const origProcProto = Object.getPrototypeOf(process);
   Object.setPrototypeOf(origProcProto, EventEmitter.prototype);
   EventEmitter.call(process);
@@ -391,7 +390,7 @@ function setupGlobalProxy() {
 }
 
 function setupBuffer() {
-  const { Buffer } = NativeModule.require('buffer');
+  const { Buffer } = require('buffer');
   const bufferBinding = internalBinding('buffer');
 
   // Only after this point can C++ use Buffer::New()
@@ -404,9 +403,9 @@ function setupBuffer() {
 
 function createGlobalConsole(consoleFromVM) {
   const consoleFromNode =
-    NativeModule.require('internal/console/global');
+    require('internal/console/global');
   if (config.hasInspector) {
-    const inspector = NativeModule.require('internal/util/inspector');
+    const inspector = require('internal/util/inspector');
     // This will be exposed by `require('inspector').console` later.
     inspector.consoleFromVM = consoleFromVM;
     // TODO(joyeecheung): postpone this until the first time inspector
@@ -424,8 +423,7 @@ function setupQueueMicrotask() {
     get() {
       process.emitWarning('queueMicrotask() is experimental.',
                           'ExperimentalWarning');
-      const { queueMicrotask } =
-        NativeModule.require('internal/queue_microtask');
+      const { queueMicrotask } = require('internal/queue_microtask');
 
       Object.defineProperty(global, 'queueMicrotask', {
         value: queueMicrotask,

Diff for: lib/internal/errors.js

@@ -1017,6 +1017,7 @@ E('ERR_UNHANDLED_ERROR',
     if (err === undefined) return msg;
     return `${msg} (${err})`;
   }, Error);
+E('ERR_UNKNOWN_BUILTIN_MODULE', 'No such built-in module: %s', Error);
 E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error);
 E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError);
 

Diff for: lib/internal/modules/cjs/loader.js

@@ -567,8 +567,8 @@ Module._resolveLookupPaths = function(request, parent, newReturn) {
 
 // Check the cache for the requested file.
 // 1. If a module already exists in the cache: return its exports object.
-// 2. If the module is native: call `NativeModule.require()` with the
-//    filename and return the result.
+// 2. If the module is native: call
+//    `NativeModule.prototype.compileForPublicLoader()` and return the exports.
 // 3. Otherwise, create a new module for the file and save it to the cache.
 //    Then have it load  the file contents before returning its exports
 //    object.
@@ -585,9 +585,10 @@ Module._load = function(request, parent, isMain) {
     return cachedModule.exports;
   }
 
-  if (NativeModule.canBeRequiredByUsers(filename)) {
+  const mod = NativeModule.map.get(filename);
+  if (mod && mod.canBeRequiredByUsers) {
     debug('load native module %s', request);
-    return NativeModule.require(filename);
+    return mod.compileForPublicLoader(experimentalModules);
   }
 
   // Don't call updateChildren(), Module constructor already does.

Diff for: lib/internal/modules/esm/translators.js

@@ -22,7 +22,9 @@ const { URL } = require('url');
 const { debuglog } = require('internal/util/debuglog');
 const { promisify } = require('internal/util');
 const esmLoader = require('internal/process/esm_loader');
-
+const {
+  ERR_UNKNOWN_BUILTIN_MODULE
+} = require('internal/errors').codes;
 const readFileAsync = promisify(fs.readFile);
 const readFileSync = fs.readFileSync;
 const StringReplace = FunctionPrototype.call.bind(StringPrototype.replace);
@@ -86,8 +88,11 @@ translators.set('builtin', async (url) => {
   debug(`Translating BuiltinModule ${url}`);
   // slice 'node:' scheme
   const id = url.slice(5);
-  NativeModule.require(id);
   const module = NativeModule.map.get(id);
+  if (!module) {
+    throw new ERR_UNKNOWN_BUILTIN_MODULE(id);
+  }
+  module.compileForPublicLoader(true);
   return createDynamicModule(
     [...module.exportKeys, 'default'], url, (reflect) => {
       debug(`Loading BuiltinModule ${url}`);

Diff for: src/node.cc

@@ -298,8 +298,6 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
       env->process_string(),
       FIXED_ONE_BYTE_STRING(isolate, "getLinkedBinding"),
       FIXED_ONE_BYTE_STRING(isolate, "getInternalBinding"),
-      // --experimental-modules
-      FIXED_ONE_BYTE_STRING(isolate, "experimentalModules"),
       // --expose-internals
       FIXED_ONE_BYTE_STRING(isolate, "exposeInternals"),
       env->primordials_string()};
@@ -311,7 +309,6 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
       env->NewFunctionTemplate(binding::GetInternalBinding)
           ->GetFunction(context)
           .ToLocalChecked(),
-      Boolean::New(isolate, env->options()->experimental_modules),
       Boolean::New(isolate, env->options()->expose_internals),
       env->primordials()};
 
@@ -333,16 +330,19 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
       loader_exports_obj->Get(context, env->require_string()).ToLocalChecked();
   env->set_native_module_require(require.As<Function>());
 
-  // process, loaderExports, isMainThread, ownsProcessState, primordials
+  // process, require, internalBinding, isMainThread,
+  // ownsProcessState, primordials
   std::vector<Local<String>> node_params = {
       env->process_string(),
-      FIXED_ONE_BYTE_STRING(isolate, "loaderExports"),
+      env->require_string(),
+      env->internal_binding_string(),
       FIXED_ONE_BYTE_STRING(isolate, "isMainThread"),
       FIXED_ONE_BYTE_STRING(isolate, "ownsProcessState"),
       env->primordials_string()};
   std::vector<Local<Value>> node_args = {
       process,
-      loader_exports_obj,
+      require,
+      internal_binding_loader,
       Boolean::New(isolate, env->is_main_thread()),
       Boolean::New(isolate, env->owns_process_state()),
       env->primordials()};