module: reduce circular dependency of internal/modules/cjs/loader

Previously `internal/bootstrap/pre_execution.js` requires
`internal/modules/cjs/loader.js` which in turn requires
`internal/bootstrap/pre_execution.js`. This patch moves the
entry point execution logic out of `pre_execution.js` and
puts it into `internal/modules/run_main.js`. It also tests
that `Module.runMain` can be monkey-patched before further
deprecation/refactoring can be done.

Also added an internal assertion `hasLoadedAnyUserCJSModule`
for documentation purposes.

PR-URL: https://github.com/nodejs/node/pull/30349
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

lib/internal/bootstrap/pre_execution.js

@@ -5,7 +5,7 @@ const { Object, SafeWeakMap } = primordials;
 const { getOptionValue } = require('internal/options');
 const { Buffer } = require('buffer');
 const { ERR_MANIFEST_ASSERT_INTEGRITY } = require('internal/errors').codes;
-const path = require('path');
+const assert = require('internal/assert');
 
 function prepareMainThreadExecution(expandArgv1 = false) {
   // Patch the process object with legacy properties and normalizations
@@ -60,6 +60,9 @@ function prepareMainThreadExecution(expandArgv1 = false) {
   initializeDeprecations();
   initializeCJSLoader();
   initializeESMLoader();
+
+  const CJSLoader = require('internal/modules/cjs/loader');
+  assert(!CJSLoader.hasLoadedAnyUserCJSModule);
   loadPreloadModules();
   initializeFrozenIntrinsics();
 }
@@ -394,7 +397,11 @@ function initializePolicy() {
 }
 
 function initializeCJSLoader() {
-  require('internal/modules/cjs/loader').Module._initPaths();
+  const CJSLoader = require('internal/modules/cjs/loader');
+  CJSLoader.Module._initPaths();
+  // TODO(joyeecheung): deprecate this in favor of a proper hook?
+  CJSLoader.Module.runMain =
+    require('internal/modules/run_main').executeUserEntryPoint;
 }
 
 function initializeESMLoader() {
@@ -433,67 +440,11 @@ function loadPreloadModules() {
   }
 }
 
-function resolveMainPath(main) {
-  const { toRealPath, Module: CJSModule } =
-      require('internal/modules/cjs/loader');
-
-  // Note extension resolution for the main entry point can be deprecated in a
-  // future major.
-  let mainPath = CJSModule._findPath(path.resolve(main), null, true);
-  if (!mainPath)
-    return;
-
-  const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
-  if (!preserveSymlinksMain)
-    mainPath = toRealPath(mainPath);
-
-  return mainPath;
-}
-
-function shouldUseESMLoader(mainPath) {
-  const userLoader = getOptionValue('--experimental-loader');
-  if (userLoader)
-    return true;
-  // Determine the module format of the main
-  if (mainPath && mainPath.endsWith('.mjs'))
-    return true;
-  if (!mainPath || mainPath.endsWith('.cjs'))
-    return false;
-  const { readPackageScope } = require('internal/modules/cjs/loader');
-  const pkg = readPackageScope(mainPath);
-  return pkg && pkg.data.type === 'module';
-}
-
-function runMainESM(mainPath) {
-  const esmLoader = require('internal/process/esm_loader');
-  const { pathToFileURL } = require('internal/url');
-  const { hasUncaughtExceptionCaptureCallback } =
-      require('internal/process/execution');
-  return esmLoader.initializeLoader().then(() => {
-    const main = path.isAbsolute(mainPath) ?
-      pathToFileURL(mainPath).href : mainPath;
-    return esmLoader.ESMLoader.import(main);
-  }).catch((e) => {
-    if (hasUncaughtExceptionCaptureCallback()) {
-      process._fatalException(e);
-      return;
-    }
-    internalBinding('errors').triggerUncaughtException(
-      e,
-      true /* fromPromise */
-    );
-  });
-}
-
-
 module.exports = {
   patchProcessObject,
-  resolveMainPath,
-  runMainESM,
   setupCoverageHooks,
   setupWarningHandler,
   setupDebugEnv,
-  shouldUseESMLoader,
   prepareMainThreadExecution,
   initializeDeprecations,
   initializeESMLoader,

lib/internal/main/run_main_module.js

@@ -6,10 +6,12 @@ const {
 
 prepareMainThreadExecution(true);
 
-const CJSModule = require('internal/modules/cjs/loader').Module;
-
 markBootstrapComplete();
 
 // Note: this loads the module through the ESM loader if the module is
-// determined to be an ES module
-CJSModule.runMain(process.argv[1]);
+// determined to be an ES module. This hangs from the CJS module loader
+// because we currently allow monkey-patching of the module loaders
+// in the preloaded scripts through require('module').
+// runMain here might be monkey-patched by users in --require.
+// XXX: the monkey-patchability here should probably be deprecated.
+require('internal/modules/cjs/loader').Module.runMain(process.argv[1]);

lib/internal/main/worker_thread.js

@@ -108,6 +108,9 @@ port.on('message', (message) => {
     initializeDeprecations();
     initializeCJSLoader();
     initializeESMLoader();
+
+    const CJSLoader = require('internal/modules/cjs/loader');
+    assert(!CJSLoader.hasLoadedAnyUserCJSModule);
     loadPreloadModules();
     initializeFrozenIntrinsics();
     publicWorker.parentPort = publicPort;
@@ -141,8 +144,9 @@ port.on('message', (message) => {
       evalScript('[worker eval]', filename);
     } else {
       // script filename
-      const CJSModule = require('internal/modules/cjs/loader').Module;
-      CJSModule.runMain(process.argv[1] = filename);
+      // runMain here might be monkey-patched by users in --require.
+      // XXX: the monkey-patchability here should probably be deprecated.
+      CJSLoader.Module.runMain(process.argv[1] = filename);
     }
   } else if (message.type === STDIO_PAYLOAD) {
     const { stream, chunk, encoding } = message;

lib/internal/modules/cjs/loader.js

@@ -64,6 +64,11 @@ const manifest = getOptionValue('--experimental-policy') ?
   require('internal/process/policy').manifest :
   null;
 const { compileFunction } = internalBinding('contextify');
+
+// Whether any user-provided CJS modules had been loaded (executed).
+// Used for internal assertions.
+let hasLoadedAnyUserCJSModule = false;
+
 const {
   ERR_INVALID_ARG_VALUE,
   ERR_INVALID_OPT_VALUE,
@@ -71,14 +76,12 @@ const {
   ERR_REQUIRE_ESM
 } = require('internal/errors').codes;
 const { validateString } = require('internal/validators');
-const {
-  resolveMainPath,
-  shouldUseESMLoader,
-  runMainESM
-} = require('internal/bootstrap/pre_execution');
 const pendingDeprecation = getOptionValue('--pending-deprecation');
 
-module.exports = { wrapSafe, Module, toRealPath, readPackageScope };
+module.exports = {
+  wrapSafe, Module, toRealPath, readPackageScope,
+  get hasLoadedAnyUserCJSModule() { return hasLoadedAnyUserCJSModule; }
+};
 
 let asyncESM, ModuleJob, ModuleWrap, kInstantiated;
 
@@ -1172,6 +1175,7 @@ Module.prototype._compile = function(content, filename) {
     result = compiledWrapper.call(thisValue, exports, require, module,
                                   filename, dirname);
   }
+  hasLoadedAnyUserCJSModule = true;
   if (requireDepth === 0) statCache = null;
   return result;
 };
@@ -1240,17 +1244,6 @@ Module._extensions['.node'] = function(module, filename) {
   return process.dlopen(module, path.toNamespacedPath(filename));
 };
 
-// Bootstrap main module.
-Module.runMain = function(main = process.argv[1]) {
-  const resolvedMain = resolveMainPath(main);
-  const useESMLoader = shouldUseESMLoader(resolvedMain);
-  if (useESMLoader) {
-    runMainESM(resolvedMain || main);
-  } else {
-    Module._load(main, null, true);
-  }
-};
-
 function createRequireFromPath(filename) {
   // Allow a directory to be passed as the filename
   const trailingSlash =

lib/internal/modules/run_main.js

@@ -0,0 +1,73 @@
+'use strict';
+
+const CJSLoader = require('internal/modules/cjs/loader');
+const { Module, toRealPath, readPackageScope } = CJSLoader;
+const { getOptionValue } = require('internal/options');
+const path = require('path');
+
+function resolveMainPath(main) {
+  // Note extension resolution for the main entry point can be deprecated in a
+  // future major.
+  // Module._findPath is monkey-patchable here.
+  let mainPath = Module._findPath(path.resolve(main), null, true);
+  if (!mainPath)
+    return;
+
+  const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
+  if (!preserveSymlinksMain)
+    mainPath = toRealPath(mainPath);
+
+  return mainPath;
+}
+
+function shouldUseESMLoader(mainPath) {
+  const userLoader = getOptionValue('--experimental-loader');
+  if (userLoader)
+    return true;
+  // Determine the module format of the main
+  if (mainPath && mainPath.endsWith('.mjs'))
+    return true;
+  if (!mainPath || mainPath.endsWith('.cjs'))
+    return false;
+  const pkg = readPackageScope(mainPath);
+  return pkg && pkg.data.type === 'module';
+}
+
+function runMainESM(mainPath) {
+  const esmLoader = require('internal/process/esm_loader');
+  const { pathToFileURL } = require('internal/url');
+  const { hasUncaughtExceptionCaptureCallback } =
+      require('internal/process/execution');
+  return esmLoader.initializeLoader().then(() => {
+    const main = path.isAbsolute(mainPath) ?
+      pathToFileURL(mainPath).href : mainPath;
+    return esmLoader.ESMLoader.import(main);
+  }).catch((e) => {
+    if (hasUncaughtExceptionCaptureCallback()) {
+      process._fatalException(e);
+      return;
+    }
+    internalBinding('errors').triggerUncaughtException(
+      e,
+      true /* fromPromise */
+    );
+  });
+}
+
+// For backwards compatibility, we have to run a bunch of
+// monkey-patchable code that belongs to the CJS loader (exposed by
+// `require('module')`) even when the entry point is ESM.
+function executeUserEntryPoint(main = process.argv[1]) {
+  const resolvedMain = resolveMainPath(main);
+  const useESMLoader = shouldUseESMLoader(resolvedMain);
+  if (useESMLoader) {
+    runMainESM(resolvedMain || main);
+  } else {
+    // Module._load is the monkey-patchable CJS module loader.
+    Module._load(main, null, true);
+  }
+}
+
+module.exports = {
+  executeUserEntryPoint
+};

node.gyp

@@ -145,6 +145,7 @@
       'lib/internal/main/run_main_module.js',
       'lib/internal/main/run_third_party_main.js',
       'lib/internal/main/worker_thread.js',
+      'lib/internal/modules/run_main.js',
       'lib/internal/modules/cjs/helpers.js',
       'lib/internal/modules/cjs/loader.js',
       'lib/internal/modules/esm/loader.js',

test/fixtures/monkey-patch-run-main.js

@@ -0,0 +1,8 @@
+'use strict';
+
+const oldRunMain = require('module').runMain;
+
+require('module').runMain = function(...args) {
+  console.log('runMain is monkey patched!');
+  oldRunMain(...args);
+};

test/message/core_line_numbers.out

@@ -10,5 +10,5 @@ RangeError: Invalid input
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
     at internal/main/run_main_module.js:*:*

test/message/error_exit.out

@@ -12,7 +12,7 @@ AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
     at internal/main/run_main_module.js:*:* {
   generatedMessage: true,
   code: 'ERR_ASSERTION',

test/message/esm_loader_not_found.out

@@ -11,8 +11,8 @@ Error: Cannot find package 'i-dont-exist' imported from *
     at Loader.import (internal/modules/esm/loader.js:*:*)
     at internal/process/esm_loader.js:*:*
     at Object.initializeLoader (internal/process/esm_loader.js:*:*)
-    at runMainESM (internal/bootstrap/pre_execution.js:*:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at runMainESM (internal/modules/run_main.js:*:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
     at internal/main/run_main_module.js:*:* {
   code: 'ERR_MODULE_NOT_FOUND'
 }

test/message/events_unhandled_error_common_trace.out

@@ -10,7 +10,7 @@ Error: foo:bar
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
     at internal/main/run_main_module.js:*:*
 Emitted 'error' event at:
     at quux (*events_unhandled_error_common_trace.js:*:*)

test/message/events_unhandled_error_nexttick.out

@@ -8,7 +8,7 @@ Error
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
     at internal/main/run_main_module.js:*:*
 Emitted 'error' event at:
     at *events_unhandled_error_nexttick.js:*:*

test/message/events_unhandled_error_sameline.out

@@ -8,7 +8,7 @@ Error
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
     at internal/main/run_main_module.js:*:*
 Emitted 'error' event at:
     at Object.<anonymous> (*events_unhandled_error_sameline.js:*:*)

test/message/events_unhandled_error_subclass.out

@@ -8,7 +8,7 @@ Error
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
     at internal/main/run_main_module.js:*:*
 Emitted 'error' event on Foo instance at:
     at Object.<anonymous> (*events_unhandled_error_subclass.js:*:*)

test/message/if-error-has-good-stack.out

@@ -15,7 +15,7 @@ AssertionError [ERR_ASSERTION]: ifError got unwanted exception: test error
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
     at internal/main/run_main_module.js:*:* {
   generatedMessage: false,
   code: 'ERR_ASSERTION',
@@ -28,7 +28,7 @@ AssertionError [ERR_ASSERTION]: ifError got unwanted exception: test error
       at Object.Module._extensions..js (internal/modules/cjs/loader.js:*:*)
       at Module.load (internal/modules/cjs/loader.js:*:*)
       at Function.Module._load (internal/modules/cjs/loader.js:*:*)
-      at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+      at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)
       at internal/main/run_main_module.js:*:*
   expected: null,
   operator: 'ifError'

test/message/throw_error_with_getter_throw_traced.out

@@ -9,4 +9,4 @@ Thrown at:
     at Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Module._load (internal/modules/cjs/loader.js:*:*)
-    at Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at executeUserEntryPoint (internal/modules/run_main.js:*:*)

test/message/throw_null_traced.out

@@ -9,4 +9,4 @@ Thrown at:
     at Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Module._load (internal/modules/cjs/loader.js:*:*)
-    at Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at executeUserEntryPoint (internal/modules/run_main.js:*:*)

test/message/throw_undefined_traced.out

@@ -9,4 +9,4 @@ Thrown at:
     at Module._extensions..js (internal/modules/cjs/loader.js:*:*)
     at Module.load (internal/modules/cjs/loader.js:*:*)
     at Module._load (internal/modules/cjs/loader.js:*:*)
-    at Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at executeUserEntryPoint (internal/modules/run_main.js:*:*)

test/message/undefined_reference_in_new_context.out

@@ -13,4 +13,4 @@ ReferenceError: foo is not defined
     at *..js (internal/modules/cjs/loader.js:*)
     at Module.load (internal/modules/cjs/loader.js:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*:*)

test/message/vm_display_runtime_error.out

@@ -12,7 +12,7 @@ Error: boo!
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
     at Module.load (internal/modules/cjs/loader.js:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*)
     at internal/main/run_main_module.js:*:*
 test.vm:1
 throw new Error("spooky!")
@@ -27,5 +27,5 @@ Error: spooky!
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
     at Module.load (internal/modules/cjs/loader.js:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*)
     at internal/main/run_main_module.js:*:*

test/message/vm_display_syntax_error.out

@@ -11,7 +11,7 @@ SyntaxError: Unexpected number
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
     at Module.load (internal/modules/cjs/loader.js:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*)
     at internal/main/run_main_module.js:*:*
 test.vm:1
 var 5;
@@ -25,5 +25,5 @@ SyntaxError: Unexpected number
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
     at Module.load (internal/modules/cjs/loader.js:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*)
     at internal/main/run_main_module.js:*:*

test/message/vm_dont_display_runtime_error.out

@@ -13,5 +13,5 @@ Error: boo!
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
     at Module.load (internal/modules/cjs/loader.js:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*)
     at internal/main/run_main_module.js:*:*

test/message/vm_dont_display_syntax_error.out

@@ -13,5 +13,5 @@ SyntaxError: Unexpected number
     at Object.Module._extensions..js (internal/modules/cjs/loader.js:*)
     at Module.load (internal/modules/cjs/loader.js:*)
     at Function.Module._load (internal/modules/cjs/loader.js:*)
-    at Function.Module.runMain (internal/modules/cjs/loader.js:*)
+    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:*)
     at internal/main/run_main_module.js:*:*

test/parallel/test-bootstrap-modules.js

@@ -46,6 +46,7 @@ const expectedModules = new Set([
   'NativeModule internal/fs/utils',
   'NativeModule internal/idna',
   'NativeModule internal/linkedlist',
+  'NativeModule internal/modules/run_main',
   'NativeModule internal/modules/cjs/helpers',
   'NativeModule internal/modules/cjs/loader',
   'NativeModule internal/modules/esm/create_dynamic_module',

test/parallel/test-module-run-main-monkey-patch.js

@@ -0,0 +1,18 @@
+'use strict';
+
+// This tests that module.runMain can be monkey patched using --require.
+// TODO(joyeecheung): This probably should be deprecated.
+
+require('../common');
+const { path } = require('../common/fixtures');
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const child = spawnSync(process.execPath, [
+  '--require',
+  path('monkey-patch-run-main.js'),
+  path('semicolon.js'),
+]);
+
+assert.strictEqual(child.status, 0);
+assert(child.stdout.toString().includes('runMain is monkey patched!'));