esm: bypass CommonJS loader under --default-type

nodejs-github-bot · Oct 5, 2023 · Oct 1, 2023 · Oct 1, 2023 · Oct 1, 2023 · Oct 1, 2023
commit 97b1966a2a2daa53f512e6d041ea6d31f147fb2b
diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js
@@ -6,18 +6,24 @@ const {
   prepareMainThreadExecution,
   markBootstrapComplete,
 } = require('internal/process/pre_execution');
+const { getOptionValue } = require('internal/options');
 
-prepareMainThreadExecution(true);
+const mainEntry = prepareMainThreadExecution(true);
 
 markBootstrapComplete();
 
 // Necessary to reset RegExp statics before user code runs.
 RegExpPrototypeExec(/^/, '');
 
-// Note: this loads the module through the ESM loader if the module is
-// 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]);
+if (getOptionValue('--experimental-default-type') === 'module') {
+  require('internal/modules/run_main').executeUserEntryPoint(mainEntry);
+} else {
+  /**
+   * To support legacy monkey-patching of `Module.runMain`, we call `runMain` here to have the CommonJS loader begin
+   * the execution of the main entry point, even if the ESM loader immediately takes over because the main entry is an
+   * ES module or one of the other opt-in conditions (such as the use of `--import`) are met. Users can monkey-patch
+   * before the main entry point is loaded by doing so via scripts loaded through `--require`. This monkey-patchability
+   * is undesirable and is removed in `--experimental-default-type=module` mode.
+   */
+  require('internal/modules/cjs/loader').Module.runMain(mainEntry);
+}
diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js
@@ -12,11 +12,16 @@ const path = require('path');
  * @param {string} main - Entry point 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.
-  const { Module } = require('internal/modules/cjs/loader');
-  let mainPath = Module._findPath(path.resolve(main), null, true);
+  /** @type {string} */
+  let mainPath;
+  if (getOptionValue('--experimental-default-type') === 'module') {
+    mainPath = path.resolve(main);
+  } else {
+    // Extension searching for the main entry point is supported only in legacy mode.
+    // Module._findPath is monkey-patchable here.
+    const { Module } = require('internal/modules/cjs/loader');
+    mainPath = Module._findPath(path.resolve(main), null, true);
+  }
   if (!mainPath) { return; }
 
   const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main');
@@ -33,6 +38,8 @@ function resolveMainPath(main) {
  * @param {string} mainPath - Absolute path to the main entry point
  */
 function shouldUseESMLoader(mainPath) {
+  if (getOptionValue('--experimental-default-type') === 'module') { return true; }
+
   /**
    * @type {string[]} userLoaders A list of custom loaders registered by the user
    * (or an empty list when none have been registered).
@@ -90,6 +97,7 @@ async function handleMainPromise(promise) {
  * Parse the CLI main entry point string and run it.
  * 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.
+ * This monkey-patchable code is bypassed under `--experimental-default-type=module`.
  * Because of backwards compatibility, this function is exposed publicly via `import { runMain } from 'node:module'`.
  * @param {string} main - Resolved absolute path for the main entry point, if found
  */

diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js
@@ -50,9 +50,10 @@ const {
     isBuildingSnapshot,
   },
 } = require('internal/v8/startup_snapshot');
+const { resolve } = require('path');
 
 function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true) {
-  prepareExecution({
+  return prepareExecution({
     expandArgv1,
     initializeModules,
     isMainThread: true,
@@ -73,8 +74,8 @@ function prepareExecution(options) {
   refreshRuntimeOptions();
   reconnectZeroFillToggle();
 
-  // Patch the process object with legacy properties and normalizations
-  patchProcessObject(expandArgv1);
+  // Patch the process object and get the resolved main entry point.
+  const mainEntry = patchProcessObject(expandArgv1);
   setupTraceCategoryState();
   setupInspectorHooks();
   setupWarningHandler();
@@ -131,6 +132,8 @@ function prepareExecution(options) {
   if (initializeModules) {
     setupUserModules();
   }
+
+  return mainEntry;
 }
 
 function setupSymbolDisposePolyfill() {
@@ -202,14 +205,17 @@ function patchProcessObject(expandArgv1) {
   process._exiting = false;
   process.argv[0] = process.execPath;
 
+  /** @type {string} */
+  let mainEntry;
   // If requested, update process.argv[1] to replace whatever the user provided with the resolved absolute file path of
   // the entry point.
   if (expandArgv1 && process.argv[1] &&
       !StringPrototypeStartsWith(process.argv[1], '-')) {
     // Expand process.argv[1] into a full path.
     const path = require('path');
     try {
-      process.argv[1] = path.resolve(process.argv[1]);
+      mainEntry = path.resolve(process.argv[1]);
+      process.argv[1] = mainEntry;
     } catch {
       // Continue regardless of error.
     }
@@ -236,6 +242,8 @@ function patchProcessObject(expandArgv1) {
   addReadOnlyProcessAlias('traceDeprecation', '--trace-deprecation');
   addReadOnlyProcessAlias('_breakFirstLine', '--inspect-brk', false);
   addReadOnlyProcessAlias('_breakNodeFirstLine', '--inspect-brk-node', false);
+
+  return mainEntry;
 }
 
 function addReadOnlyProcessAlias(name, option, enumerable = true) {

diff --git a/test/es-module/test-esm-type-flag-errors.mjs b/test/es-module/test-esm-type-flag-errors.mjs
@@ -3,6 +3,36 @@ import * as fixtures from '../common/fixtures.mjs';
 import { describe, it } from 'node:test';
 import { match, strictEqual } from 'node:assert';
 
+describe('--experimental-default-type=module should not support extension searching', { concurrency: true }, () => {
+  it('should support extension searching under --experimental-default-type=commonjs', async () => {
+    const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
+      '--experimental-default-type=commonjs',
+      './index',
+    ], {
+      cwd: fixtures.path('es-modules/package-without-type'),
+    });
+
+    strictEqual(stdout, 'package-without-type\n');
+    strictEqual(stderr, '');
+    strictEqual(code, 0);
+    strictEqual(signal, null);
+  });
+
+  it('should error with implicit extension under --experimental-default-type=module', async () => {
+    const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [
+      '--experimental-default-type=module',
+      './index',
+    ], {
+      cwd: fixtures.path('es-modules/package-without-type'),
+    });
+
+    match(stderr, /ENOENT/);
+    strictEqual(stdout, '');
+    strictEqual(code, 1);
+    strictEqual(signal, null);
+  });
+});
+
 describe('--experimental-default-type=module should not affect the interpretation of files with unknown extensions',
          { concurrency: true }, () => {
            it('should error on an entry point with an unknown extension', async () => {