module: refactor ESM loader initialization and entry point handling

Split the `internal/process/esm_loader` file which contains the singleton cascaded loader: - The the singleton cascaded loader now directly resides in `internal/modules/esm/loader`, where the constructor also lives. This file is the root of most circular dependency of ESM code, (because components of the loader need the singleton itself), so this makes the dependency more obvious. Added comments about loading it lazily to avoid circular dependency. - The getter to the cascaded loader is also turned into a method to make the side effect explicit. - The sequence of `loadESM()` and `handleMainPromise` is now merged together into `runEntryPointWithESMLoader()` in `internal/modules/run_main` because this is intended to run entry points with the ESM loader and not just any module. - Documents how top-level await is handled. PR-URL: nodejs#51999 Fixes: nodejs#42868 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
jcbhmr · May 15, 2024 · 2ffdba4 · 2ffdba4
1 parent 83a5668
commit 2ffdba4
Show file tree

Hide file tree

Showing 17 changed files with 136 additions and 140 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -96,7 +96,6 @@
 /doc/api/packages.md @nodejs/loaders
 /lib/internal/bootstrap/realm.js @nodejs/loaders
 /lib/internal/modules/* @nodejs/loaders
-/lib/internal/process/esm_loader.js @nodejs/loaders
 /lib/internal/process/execution.js @nodejs/loaders
 /lib/module.js @nodejs/loaders
 /src/module_wrap* @nodejs/loaders @nodejs/vm

diff --git a/lib/internal/main/check_syntax.js b/lib/internal/main/check_syntax.js
@@ -50,8 +50,7 @@ function loadESMIfNeeded(cb) {
   const hasModulePreImport = getOptionValue('--import').length > 0;
 
   if (hasModulePreImport) {
-    const { loadESM } = require('internal/process/esm_loader');
-    loadESM(cb);
+    require('internal/modules/run_main').runEntryPointWithESMLoader(cb);
     return;
   }
   cb();
@@ -76,7 +75,5 @@ async function checkSyntax(source, filename) {
     return;
   }
 
-  const { loadESM } = require('internal/process/esm_loader');
-  const { handleMainPromise } = require('internal/modules/run_main');
-  handleMainPromise(loadESM((loader) => wrapSafe(filename, source)));
+  wrapSafe(filename, source);
 }
diff --git a/lib/internal/main/eval_stdin.js b/lib/internal/main/eval_stdin.js
@@ -10,7 +10,7 @@ const {
 const { getOptionValue } = require('internal/options');
 
 const {
-  evalModule,
+  evalModuleEntryPoint,
   evalScript,
   readStdin,
 } = require('internal/process/execution');
@@ -24,15 +24,15 @@ readStdin((code) => {
   process._eval = code;
 
   const print = getOptionValue('--print');
-  const loadESM = getOptionValue('--import').length > 0;
+  const shouldLoadESM = getOptionValue('--import').length > 0;
   if (getOptionValue('--input-type') === 'module' ||
     (getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs')) {
-    evalModule(code, print);
+    evalModuleEntryPoint(code, print);
   } else {
     evalScript('[stdin]',
                code,
                getOptionValue('--inspect-brk'),
                print,
-               loadESM);
+               shouldLoadESM);
   }
 });
diff --git a/lib/internal/main/eval_string.js b/lib/internal/main/eval_string.js
@@ -13,7 +13,7 @@ const {
   prepareMainThreadExecution,
   markBootstrapComplete,
 } = require('internal/process/pre_execution');
-const { evalModule, evalScript } = require('internal/process/execution');
+const { evalModuleEntryPoint, evalScript } = require('internal/process/execution');
 const { addBuiltinLibsToObject } = require('internal/modules/helpers');
 
 const { getOptionValue } = require('internal/options');
@@ -24,10 +24,10 @@ markBootstrapComplete();
 
 const source = getOptionValue('--eval');
 const print = getOptionValue('--print');
-const loadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0;
+const shouldLoadESM = getOptionValue('--import').length > 0 || getOptionValue('--experimental-loader').length > 0;
 if (getOptionValue('--input-type') === 'module' ||
   (getOptionValue('--experimental-default-type') === 'module' && getOptionValue('--input-type') !== 'commonjs')) {
-  evalModule(source, print);
+  evalModuleEntryPoint(source, print);
 } else {
   // For backward compatibility, we want the identifier crypto to be the
   // `node:crypto` module rather than WebCrypto.
@@ -54,5 +54,5 @@ if (getOptionValue('--input-type') === 'module' ||
              ) : source,
              getOptionValue('--inspect-brk'),
              print,
-             loadESM);
+             shouldLoadESM);
 }
diff --git a/lib/internal/main/repl.js b/lib/internal/main/repl.js
@@ -35,8 +35,7 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
     process.exit(kInvalidCommandLineArgument);
   }
 
-  const esmLoader = require('internal/process/esm_loader');
-  esmLoader.loadESM(() => {
+  require('internal/modules/run_main').runEntryPointWithESMLoader(() => {
     console.log(`Welcome to Node.js ${process.version}.\n` +
       'Type ".help" for more information.');
 
@@ -64,5 +63,7 @@ if (process.env.NODE_REPL_EXTERNAL_MODULE) {
                  getOptionValue('--inspect-brk'),
                  getOptionValue('--print'));
     }
+    // The TLAs in the REPL are still run as scripts, just transformed as async
+    // IIFEs for the REPL code itself to await on.
   });
 }
diff --git a/lib/internal/main/worker_thread.js b/lib/internal/main/worker_thread.js
@@ -170,8 +170,8 @@ port.on('message', (message) => {
       }
 
       case 'module': {
-        const { evalModule } = require('internal/process/execution');
-        PromisePrototypeThen(evalModule(filename), undefined, (e) => {
+        const { evalModuleEntryPoint } = require('internal/process/execution');
+        PromisePrototypeThen(evalModuleEntryPoint(filename), undefined, (e) => {
           workerOnGlobalUncaughtException(e, true);
         });
         break;

diff --git a/lib/internal/modules/esm/handle_process_exit.js b/lib/internal/modules/esm/handle_process_exit.js
diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
@@ -146,8 +146,8 @@ class Hooks {
    * loader (user-land) to the worker.
    */
   async register(urlOrSpecifier, parentURL, data) {
-    const moduleLoader = require('internal/process/esm_loader').esmLoader;
-    const keyedExports = await moduleLoader.import(
+    const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
+    const keyedExports = await cascadedLoader.import(
       urlOrSpecifier,
       parentURL,
       kEmptyObject,

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -20,7 +20,7 @@ const {
   ERR_UNKNOWN_MODULE_FORMAT,
 } = require('internal/errors').codes;
 const { getOptionValue } = require('internal/options');
-const { pathToFileURL, isURL } = require('internal/url');
+const { isURL } = require('internal/url');
 const { emitExperimentalWarning } = require('internal/util');
 const {
   getDefaultConditions,
@@ -85,11 +85,6 @@ class ModuleLoader {
    */
   #defaultConditions = getDefaultConditions();
 
-  /**
-   * The index for assigning unique URLs to anonymous module evaluation
-   */
-  evalIndex = 0;
-
   /**
    * Registry of resolved specifiers
    */
@@ -187,10 +182,7 @@ class ModuleLoader {
     }
   }
 
-  async eval(
-    source,
-    url = pathToFileURL(`${process.cwd()}/[eval${++this.evalIndex}]`).href,
-  ) {
+  async eval(source, url) {
     const evalInstance = (url) => {
       const { ModuleWrap } = internalBinding('module_wrap');
       const { registerModule } = require('internal/modules/esm/utils');
@@ -214,6 +206,7 @@ class ModuleLoader {
     return {
       __proto__: null,
       namespace: module.getNamespace(),
+      module,
     };
   }
 
@@ -568,6 +561,23 @@ function getHooksProxy() {
   return hooksProxy;
 }
 
+let cascadedLoader;
+
+/**
+ * This is a singleton ESM loader that integrates the loader hooks, if any.
+ * It it used by other internal built-ins when they need to load ESM code
+ * while also respecting hooks.
+ * When built-ins need access to this loader, they should do
+ * require('internal/module/esm/loader').getOrInitializeCascadedLoader()
+ * lazily only right before the loader is actually needed, and don't do it
+ * in the top-level, to avoid circular dependencies.
+ * @returns {ModuleLoader}
+ */
+function getOrInitializeCascadedLoader() {
+  cascadedLoader ??= createModuleLoader();
+  return cascadedLoader;
+}
+
 /**
  * Register a single loader programmatically.
  * @param {string|import('url').URL} specifier
@@ -598,12 +608,11 @@ function getHooksProxy() {
  * ```
  */
 function register(specifier, parentURL = undefined, options) {
-  const moduleLoader = require('internal/process/esm_loader').esmLoader;
   if (parentURL != null && typeof parentURL === 'object' && !isURL(parentURL)) {
     options = parentURL;
     parentURL = options.parentURL;
   }
-  moduleLoader.register(
+  getOrInitializeCascadedLoader().register(
     specifier,
     parentURL ?? 'data:',
     options?.data,
@@ -614,5 +623,6 @@ function register(specifier, parentURL = undefined, options) {
 module.exports = {
   createModuleLoader,
   getHooksProxy,
+  getOrInitializeCascadedLoader,
   register,
 };
diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
@@ -55,7 +55,6 @@ const {
 const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
 const moduleWrap = internalBinding('module_wrap');
 const { ModuleWrap } = moduleWrap;
-const asyncESM = require('internal/process/esm_loader');
 const { emitWarningSync } = require('internal/process/warning');
 const { internalCompileFunction } = require('internal/vm');
 const {
@@ -157,7 +156,8 @@ function errPath(url) {
  * @returns {Promise<import('internal/modules/esm/loader.js').ModuleExports>} The imported module.
  */
 async function importModuleDynamically(specifier, { url }, attributes) {
-  return asyncESM.esmLoader.import(specifier, url, attributes);
+  const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
+  return cascadedLoader.import(specifier, url, attributes);
 }
 
 // Strategy for loading a standard JavaScript module.
@@ -243,6 +243,7 @@ function loadCJSModule(module, source, url, filename) {
 
   const compiledWrapper = compileResult.function;
 
+  const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
   const __dirname = dirname(filename);
   // eslint-disable-next-line func-name-matching,func-style
   const requireFn = function require(specifier) {
@@ -261,7 +262,7 @@ function loadCJSModule(module, source, url, filename) {
       }
       specifier = `${pathToFileURL(path)}`;
     }
-    const job = asyncESM.esmLoader.getModuleJobSync(specifier, url, importAttributes);
+    const job = cascadedLoader.getModuleJobSync(specifier, url, importAttributes);
     job.runSync();
     return cjsCache.get(job.url).exports;
   };
@@ -272,7 +273,7 @@ function loadCJSModule(module, source, url, filename) {
         specifier = `${pathToFileURL(path)}`;
       }
     }
-    const { url: resolvedURL } = asyncESM.esmLoader.resolveSync(specifier, url, kEmptyObject);
+    const { url: resolvedURL } = cascadedLoader.resolveSync(specifier, url, kEmptyObject);
     return StringPrototypeStartsWith(resolvedURL, 'file://') ? fileURLToPath(resolvedURL) : resolvedURL;
   });
   setOwnProperty(requireFn, 'main', process.mainModule);

diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js
@@ -32,7 +32,6 @@ const {
 const {
   emitExperimentalWarning,
   getCWDURL,
-  getLazy,
 } = require('internal/util');
 const {
   setImportModuleDynamicallyCallback,
@@ -181,9 +180,6 @@ function initializeImportMetaObject(symbol, meta) {
     }
   }
 }
-const getCascadedLoader = getLazy(
-  () => require('internal/process/esm_loader').esmLoader,
-);
 
 /**
  * Proxy the dynamic import to the default loader.
@@ -194,7 +190,8 @@ const getCascadedLoader = getLazy(
  */
 function defaultImportModuleDynamically(specifier, attributes, referrerName) {
   const parentURL = normalizeReferrerURL(referrerName);
-  return getCascadedLoader().import(specifier, parentURL, attributes);
+  const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
+  return cascadedLoader.import(specifier, parentURL, attributes);
 }
 
 /**
@@ -263,10 +260,10 @@ async function initializeHooks() {
   const customLoaderURLs = getOptionValue('--experimental-loader');
 
   const { Hooks } = require('internal/modules/esm/hooks');
-  const esmLoader = require('internal/process/esm_loader').esmLoader;
+  const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
 
   const hooks = new Hooks();
-  esmLoader.setCustomizations(hooks);
+  cascadedLoader.setCustomizations(hooks);
 
   // We need the loader customizations to be set _before_ we start invoking
   // `--require`, otherwise loops can happen because a `--require` script