esm: use sync loading/resolving on non-loader-hook thread

ESM resolution and loading is now always synchronous from a
non-loader-hook thread. If no asynchrnous loader hooks are
registered, the resolution/loading is entirely synchronous.
If asynchronous loader hooks are registered, these would be
synchronous on the non-loader-hook thread, and asynchronous
on the loader hook thread.

This avoids several races caused by async/sync loading sharing
the same cache. In particular, asynchronous loader hooks
now works with `require(esm)` - previously it tends to break
due to races.

In addition, when an asynchronous loader hook
returns a promise that never settles, the main thread no longer
silently exits with exit code 13, leaving the code below
any module loading calls silently ignored without being executed.
Instead, it now throws ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED
which can be caught and handled by the main thread. If the module
request comes from `import()`, the never-settling promise is
now relayed to the result returned by `import()`.

Drive-by: when annotating the error about importing undetectable
named exports from CommonJS, it now no longer reload the source
code of the CommonJS module, and instead reuses format information
cached when the module was loaded for linking.

PR-URL: https://github.com/nodejs/node/pull/60380
Fixes: https://github.com/nodejs/node/issues/59666
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Jacob Smith <jacob@frende.me>

doc/api/errors.md

@@ -703,6 +703,13 @@ by the `node:assert` module.
 An attempt was made to register something that is not a function as an
 `AsyncHooks` callback.
 
+<a id="ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED"></a>
+
+### `ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED`
+
+An operation related to module loading is customized by an asynchronous loader
+hook that never settled the promise before the loader thread exits.
+
 <a id="ERR_ASYNC_TYPE"></a>
 
 ### `ERR_ASYNC_TYPE`

lib/internal/errors.js

@@ -1137,6 +1137,8 @@ E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError);
 E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError);
 E('ERR_ASSERTION', '%s', Error);
 E('ERR_ASYNC_CALLBACK', '%s must be a function', TypeError);
+E('ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED',
+  'Async loader request never settled', Error);
 E('ERR_ASYNC_TYPE', 'Invalid name for async "type": %s', TypeError);
 E('ERR_BROTLI_INVALID_PARAM', '%s is not a valid Brotli parameter', RangeError);
 E('ERR_BUFFER_OUT_OF_BOUNDS',

lib/internal/modules/esm/hooks.js

@@ -23,16 +23,15 @@ const {
 } = globalThis;
 
 const {
+  ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED,
   ERR_INTERNAL_ASSERTION,
   ERR_INVALID_ARG_TYPE,
   ERR_INVALID_ARG_VALUE,
   ERR_INVALID_RETURN_PROPERTY_VALUE,
   ERR_INVALID_RETURN_VALUE,
   ERR_LOADER_CHAIN_INCOMPLETE,
-  ERR_METHOD_NOT_IMPLEMENTED,
   ERR_WORKER_UNSERIALIZABLE_ERROR,
 } = require('internal/errors').codes;
-const { exitCodes: { kUnsettledTopLevelAwait } } = internalBinding('errors');
 const { URLParse } = require('internal/url');
 const { canParse: URLCanParse } = internalBinding('url');
 const { receiveMessageOnPort } = require('worker_threads');
@@ -117,15 +116,16 @@ function defineImportAssertionAlias(context) {
  * via `ModuleLoader.#setAsyncLoaderHooks()`.
  * @typedef {object} AsyncLoaderHooks
  * @property {boolean} allowImportMetaResolve Whether to allow the use of `import.meta.resolve`.
+ * @property {boolean} isForAsyncLoaderHookWorker Whether the instance is running on the loader hook worker thread.
  * @property {(url: string, context: object, defaultLoad: Function) => Promise<LoadResult>} load
  *   Calling the asynchronous `load` hook asynchronously.
- * @property {(url: string, context: object, defaultLoad: Function) => LoadResult} loadSync
+ * @property {(url: string, context: object, defaultLoad: Function) => LoadResult} [loadSync]
  *   Calling the asynchronous `load` hook synchronously.
  * @property {(originalSpecifier: string, parentURL: string,
  *             importAttributes: Record<string, string>) => Promise<ResolveResult>} resolve
  *   Calling the asynchronous `resolve` hook asynchronously.
  * @property {(originalSpecifier: string, parentURL: string,
- *            importAttributes: Record<string, string>) => ResolveResult} resolveSync
+ *            importAttributes: Record<string, string>) => ResolveResult} [resolveSync]
  *   Calling the asynchronous `resolve` hook synchronously.
  * @property {(specifier: string, parentURL: string) => any} register Register asynchronous loader hooks
  * @property {() => void} waitForLoaderHookInitialization Force loading of hooks.
@@ -169,6 +169,8 @@ class AsyncLoaderHooksOnLoaderHookWorker {
 
   allowImportMetaResolve = false;
 
+  isForAsyncLoaderHookWorker = true;
+
   /**
    * Import and register custom/user-defined module loader hook(s).
    * @param {string} urlOrSpecifier
@@ -350,10 +352,6 @@ class AsyncLoaderHooksOnLoaderHookWorker {
     };
   }
 
-  resolveSync(_originalSpecifier, _parentURL, _importAttributes) {
-    throw new ERR_METHOD_NOT_IMPLEMENTED('resolveSync()');
-  }
-
   /**
    * Provide source that is understood by one of Node's translators.
    *
@@ -560,7 +558,10 @@ class AsyncLoaderHookWorker {
         debug('wait for signal from worker');
         AtomicsWait(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION, 0);
         const response = this.#worker.receiveMessageSync();
-        if (response == null || response.message.status === 'exit') { return; }
+        if (response == null) { return; }
+        if (response.message.status === 'exit') {
+          process.exit(response.message.body);
+        }
 
         // ! This line catches initialization errors in the worker thread.
         this.#unwrapMessage(response);
@@ -647,10 +648,13 @@ class AsyncLoaderHookWorker {
       this.#workerNotificationLastId = AtomicsLoad(this.#lock, WORKER_TO_MAIN_THREAD_NOTIFICATION);
 
       response = this.#worker.receiveMessageSync();
+      debug('got sync message from worker', { method, args, response });
     } while (response == null);
-    debug('got sync response from worker', { method, args });
+
     if (response.message.status === 'never-settle') {
-      process.exit(kUnsettledTopLevelAwait);
+      const error = new ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED();
+      error.details = { method, args };
+      throw error;
     } else if (response.message.status === 'exit') {
       process.exit(response.message.body);
     }
@@ -819,6 +823,8 @@ class AsyncLoaderHooksProxiedToLoaderHookWorker {
 
   allowImportMetaResolve = true;
 
+  isForAsyncLoaderHookWorker = false;
+
   /**
    * Instantiate a module loader that uses user-provided custom loader hooks.
    */

lib/internal/modules/esm/initialize_import_meta.js

@@ -33,7 +33,8 @@ function createImportMetaResolve(defaultParentURL, loader, allowParentURL) {
     }
 
     try {
-      ({ url } = loader.resolveSync(specifier, parentURL));
+      const request = { specifier, __proto__: null };
+      ({ url } = loader.resolveSync(parentURL, request));
       return url;
     } catch (error) {
       switch (error?.code) {

lib/internal/modules/esm/loader.js

@@ -7,6 +7,8 @@ const {
   FunctionPrototypeCall,
   JSONStringify,
   ObjectSetPrototypeOf,
+  Promise,
+  PromisePrototypeThen,
   RegExpPrototypeSymbolReplace,
   encodeURIComponent,
   hardenRegExp,
@@ -25,17 +27,20 @@ const {
   ERR_REQUIRE_ASYNC_MODULE,
   ERR_REQUIRE_CYCLE_MODULE,
   ERR_REQUIRE_ESM,
-  ERR_NETWORK_IMPORT_DISALLOWED,
   ERR_UNKNOWN_MODULE_FORMAT,
 } = require('internal/errors').codes;
 const { getOptionValue } = require('internal/options');
-const { isURL, pathToFileURL, URLParse } = require('internal/url');
+const { isURL, pathToFileURL } = require('internal/url');
 const { kEmptyObject } = require('internal/util');
 const {
   compileSourceTextModule,
   getDefaultConditions,
   shouldSpawnLoaderHookWorker,
+  requestTypes: { kImportInRequiredESM, kRequireInImportedCJS, kImportInImportedESM },
 } = require('internal/modules/esm/utils');
+/**
+ * @typedef {import('./utils.js').ModuleRequestType} ModuleRequestType
+ */
 const { kImplicitTypeAttribute } = require('internal/modules/esm/assert');
 const {
   ModuleWrap,
@@ -111,15 +116,16 @@ function getTranslators() {
  * async linking.
  * @param {string} filename Filename of the module being required.
  * @param {string|undefined} parentFilename Filename of the module calling require().
+ * @param {boolean} isForAsyncLoaderHookWorker Whether this is for the async loader hook worker.
  * @returns {string} Error message.
  */
-function getRaceMessage(filename, parentFilename) {
+function getRaceMessage(filename, parentFilename, isForAsyncLoaderHookWorker) {
-  let raceMessage = `Cannot require() ES Module ${filename} because it is not yet fully loaded. `;
+  let raceMessage = `Cannot require() ES Module ${filename} because it is not yet fully loaded.\n`;
   raceMessage += 'This may be caused by a race condition if the module is simultaneously dynamically ';
-  raceMessage += 'import()-ed via Promise.all(). Try await-ing the import() sequentially in a loop instead.';
+  raceMessage += 'import()-ed via Promise.all().\n';
-  if (parentFilename) {
+  raceMessage += 'Try await-ing the import() sequentially in a loop instead.\n';
-    raceMessage += ` (from ${parentFilename})`;
+  raceMessage += ` (From ${parentFilename ? `${parentFilename} in ` : ' '}`;
-  }
+  raceMessage += `${isForAsyncLoaderHookWorker ? 'loader hook worker thread' : 'non-loader-hook thread'})`;
   return raceMessage;
 }
 
@@ -184,6 +190,12 @@ class ModuleLoader {
    */
   allowImportMetaResolve;
 
+  /**
+   * @see {AsyncLoaderHooks.isForAsyncLoaderHookWorker}
+   * Shortcut to this.#asyncLoaderHooks.isForAsyncLoaderHookWorker.
+   */
+  isForAsyncLoaderHookWorker = false;
+
   /**
    * Asynchronous loader hooks to pass requests to.
    *
@@ -223,8 +235,10 @@ class ModuleLoader {
     this.#asyncLoaderHooks = asyncLoaderHooks;
     if (asyncLoaderHooks) {
       this.allowImportMetaResolve = asyncLoaderHooks.allowImportMetaResolve;
+      this.isForAsyncLoaderHookWorker = asyncLoaderHooks.isForAsyncLoaderHookWorker;
     } else {
       this.allowImportMetaResolve = true;
+      this.isForAsyncLoaderHookWorker = false;
     }
   }
 
@@ -249,7 +263,7 @@ class ModuleLoader {
   async executeModuleJob(url, wrap, isEntryPoint = false) {
     const { ModuleJob } = require('internal/modules/esm/module_job');
     const module = await onImport.tracePromise(async () => {
-      const job = new ModuleJob(this, url, undefined, wrap, kEvaluationPhase, false, false);
+      const job = new ModuleJob(this, url, undefined, wrap, kEvaluationPhase, false, false, kImportInImportedESM);
       this.loadCache.set(url, undefined, job);
       const { module } = await job.run(isEntryPoint);
       return module;
@@ -279,62 +293,6 @@ class ModuleLoader {
     return this.executeModuleJob(url, wrap, isEntryPoint);
   }
 
-  /**
-   * Get a (possibly not yet fully linked) module job from the cache, or create one and return its Promise.
-   * @param {string} specifier The module request of the module to be resolved. Typically, what's
-   *   requested by `import '<specifier>'` or `import('<specifier>')`.
-   * @param {string} [parentURL] The URL of the module where the module request is initiated.
-   *   It's undefined if it's from the root module.
-   * @param {ImportAttributes} importAttributes Attributes from the import statement or expression.
-   * @param {number} phase Import phase.
-   * @returns {Promise<ModuleJobBase>}
-   */
-  async getModuleJobForImport(specifier, parentURL, importAttributes, phase) {
-    const resolveResult = await this.resolve(specifier, parentURL, importAttributes);
-    return this.#getJobFromResolveResult(resolveResult, parentURL, importAttributes, phase, false);
-  }
-
-  /**
-   * Similar to {@link getModuleJobForImport} but it's used for `require()` resolved by the ESM loader
-   * in imported CJS modules. This runs synchronously and when it returns, the module job's module
-   * requests are all linked.
-   * @param {string} specifier See {@link getModuleJobForImport}
-   * @param {string} [parentURL] See {@link getModuleJobForImport}
-   * @param {ImportAttributes} importAttributes See {@link getModuleJobForImport}
-   * @param {number} phase Import phase.
-   * @returns {Promise<ModuleJobBase>}
-   */
-  getModuleJobForRequireInImportedCJS(specifier, parentURL, importAttributes, phase) {
-    const resolveResult = this.resolveSync(specifier, parentURL, importAttributes);
-    return this.#getJobFromResolveResult(resolveResult, parentURL, importAttributes, phase, true);
-  }
-
-  /**
-   * Given a resolved module request, obtain a ModuleJobBase from it - if it's already cached,
-   * return the cached ModuleJobBase. Otherwise, load its source and translate it into a ModuleWrap first.
-   * @param {{ format: string, url: string }} resolveResult Resolved module request.
-   * @param {string} [parentURL] See {@link getModuleJobForImport}
-   * @param {ImportAttributes} importAttributes See {@link getModuleJobForImport}
-   * @param {number} phase Import phase.
-   * @param {boolean} isForRequireInImportedCJS Whether this is done for require() in imported CJS.
-   * @returns {ModuleJobBase}
-   */
-  #getJobFromResolveResult(resolveResult, parentURL, importAttributes, phase,
-                           isForRequireInImportedCJS = false) {
-    const { url, format } = resolveResult;
-    const resolvedImportAttributes = resolveResult.importAttributes ?? importAttributes;
-    let job = this.loadCache.get(url, resolvedImportAttributes.type);
-
-    if (job === undefined) {
-      job = this.#createModuleJob(url, resolvedImportAttributes, phase, parentURL, format,
-                                  isForRequireInImportedCJS);
-    } else {
-      job.ensurePhase(phase);
-    }
-
-    return job;
-  }
-
   /**
    * This constructs (creates, instantiates and evaluates) a module graph that
    * is require()'d.
@@ -347,6 +305,10 @@ class ModuleLoader {
    */
   importSyncForRequire(mod, filename, source, isMain, parent) {
     const url = pathToFileURL(filename).href;
+    if (!getOptionValue('--experimental-require-module')) {
+      throw new ERR_REQUIRE_ESM(url, true);
+    }
+
     let job = this.loadCache.get(url, kImplicitTypeAttribute);
     // This module job is already created:
     // 1. If it was loaded by `require()` before, at this point the instantiation
@@ -364,9 +326,9 @@ class ModuleLoader {
     if (job !== undefined) {
       mod[kRequiredModuleSymbol] = job.module;
       const parentFilename = urlToFilename(parent?.filename);
-      // TODO(node:55782): this race may stop to happen when the ESM resolution and loading become synchronous.
+      // This race should only be possible on the loader hook thread. See https://github.com/nodejs/node/issues/59666
       if (!job.module) {
-        assert.fail(getRaceMessage(filename, parentFilename));
+        assert.fail(getRaceMessage(filename, parentFilename), this.isForAsyncLoaderHookWorker);
       }
       const status = job.module.getStatus();
       debug('Module status', job, status);
@@ -419,92 +381,67 @@ class ModuleLoader {
     const inspectBrk = (isMain && getOptionValue('--inspect-brk'));
 
     const { ModuleJobSync } = require('internal/modules/esm/module_job');
-    job = new ModuleJobSync(this, url, kEmptyObject, wrap, kEvaluationPhase, isMain, inspectBrk);
+    job = new ModuleJobSync(this, url, kEmptyObject, wrap, kEvaluationPhase, isMain, inspectBrk,
+                            kImportInRequiredESM);
     this.loadCache.set(url, kImplicitTypeAttribute, job);
     mod[kRequiredModuleSymbol] = job.module;
     return { wrap: job.module, namespace: job.runSync(parent).namespace };
   }
 
   /**
-   * Resolve individual module requests and create or get the cached ModuleWraps for
+   * Check invariants on a cached module job when require()'d from ESM.
-   * each of them. This is only used to create a module graph being require()'d.
+   * @param {string} specifier The first parameter of require().
-   * @param {string} specifier Specifier of the the imported module.
+   * @param {string} url URL of the module being required.
-   * @param {string} parentURL Where the import comes from.
+   * @param {string|undefined} parentURL URL of the module calling require().
-   * @param {object} importAttributes import attributes from the import statement.
+   * @param {ModuleJobBase} job The cached module job.
-   * @param {number} phase The import phase.
-   * @returns {ModuleJobBase}
    */
-  getModuleJobForRequire(specifier, parentURL, importAttributes, phase) {
+  #checkCachedJobForRequireESM(specifier, url, parentURL, job) {
-    const parsed = URLParse(specifier);
+    // This race should only be possible on the loader hook thread. See https://github.com/nodejs/node/issues/59666
-    if (parsed != null) {
+    if (!job.module) {
-      const protocol = parsed.protocol;
+      assert.fail(getRaceMessage(url, parentURL, this.isForAsyncLoaderHookWorker));
-      if (protocol === 'https:' || protocol === 'http:') {
+    }
-        throw new ERR_NETWORK_IMPORT_DISALLOWED(specifier, parentURL,
+    // This module is being evaluated, which means it's imported in a previous link
-                                                'ES modules cannot be loaded by require() from the network');
+    // in a cycle.
+    if (job.module.getStatus() === kEvaluating) {
+      const parentFilename = urlToFilename(parentURL);
+      let message = `Cannot import Module ${specifier} in a cycle.`;
+      if (parentFilename) {
+        message += ` (from ${parentFilename})`;
       }
-      assert(protocol === 'file:' || protocol === 'node:' || protocol === 'data:');
+      throw new ERR_REQUIRE_CYCLE_MODULE(message);
     }
 
-    // TODO(joyeecheung): consolidate cache behavior and use resolveSync() and
+    // Otherwise the module could be imported before but the evaluation may be already
-    // loadSync() here.
+    // completed (e.g. the require call is lazy) so it's okay. We will return the
-    const resolveResult = this.#cachedResolveSync(specifier, parentURL, importAttributes);
+    // job and check asynchronicity of the entire graph later, after the
-    const { url, format } = resolveResult;
+    // graph is instantiated.
-    if (!getOptionValue('--experimental-require-module')) {
+  }
-      throw new ERR_REQUIRE_ESM(url, true);
-    }
-    const resolvedImportAttributes = resolveResult.importAttributes ?? importAttributes;
-    let job = this.loadCache.get(url, resolvedImportAttributes.type);
-    if (job !== undefined) {
-      // TODO(node:55782): this race may stop happening when the ESM resolution and loading become synchronous.
-      if (!job.module) {
-        assert.fail(getRaceMessage(url, parentURL));
-      }
-      // This module is being evaluated, which means it's imported in a previous link
-      // in a cycle.
-      if (job.module.getStatus() === kEvaluating) {
-        const parentFilename = urlToFilename(parentURL);
-        let message = `Cannot import Module ${specifier} in a cycle.`;
-        if (parentFilename) {
-          message += ` (from ${parentFilename})`;
-        }
-        throw new ERR_REQUIRE_CYCLE_MODULE(message);
-      }
-      job.ensurePhase(phase);
-      // Otherwise the module could be imported before but the evaluation may be already
-      // completed (e.g. the require call is lazy) so it's okay. We will return the
-      // module now and check asynchronicity of the entire graph later, after the
-      // graph is instantiated.
-      return job;
-    }
 
-    const loadResult = this.#loadSync(url, { format, importAttributes });
+  /**
-
+   * Load a module and translate it into a ModuleWrap for require(esm).
-    const formatFromLoad = loadResult.format;
+   * This is run synchronously, and the translator always return a ModuleWrap synchronously.
+   * @param {string} url URL of the module to be translated.
+   * @param {object} loadContext See {@link load}
+   * @param {string|undefined} parentURL URL of the parent module. Undefined if it's the entry point.
+   * @param {ModuleRequest} request Module request.
+   * @returns {ModuleWrap}
+   */
+  loadAndTranslateForImportInRequiredESM(url, loadContext, parentURL, request) {
+    const loadResult = this.#loadSync(url, loadContext);
     // Use the synchronous commonjs translator which can deal with cycles.
+    const formatFromLoad = loadResult.format;
     const translatorKey = (formatFromLoad === 'commonjs' || formatFromLoad === 'commonjs-typescript') ?
       'commonjs-sync' : formatFromLoad;
-
+    const translateContext = { ...loadResult, translatorKey, __proto__: null };
-    if (translatorKey === 'wasm') {
-      assert.fail('WASM is currently unsupported by require(esm)');
-    }
-
-    const { source } = loadResult;
-    const isMain = (parentURL === undefined);
-    const translateContext = { format: formatFromLoad, source, translatorKey, __proto__: null };
     const wrap = this.#translate(url, translateContext, parentURL);
     assert(wrap instanceof ModuleWrap, `Translator used for require(${url}) should not be async`);
 
-    if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
-      process.send({ 'watch:import': [url] });
-    }
-
     const cjsModule = wrap[imported_cjs_symbol];
     if (cjsModule) {
-      assert(translatorKey === 'commonjs-sync');
       // Check if the ESM initiating import CJS is being required by the same CJS module.
       if (cjsModule?.[kIsExecuting]) {
         const parentFilename = urlToFilename(parentURL);
-        let message = `Cannot import CommonJS Module ${specifier} in a cycle.`;
+        let message = `Cannot import CommonJS Module ${request.specifier} in a cycle.`;
         if (parentFilename) {
           message += ` (from ${parentFilename})`;
         }
@@ -512,12 +449,7 @@ class ModuleLoader {
       }
     }
 
-    const inspectBrk = (isMain && getOptionValue('--inspect-brk'));
+    return wrap;
-    const { ModuleJobSync } = require('internal/modules/esm/module_job');
-    job = new ModuleJobSync(this, url, importAttributes, wrap, phase, isMain, inspectBrk);
-
-    this.loadCache.set(url, importAttributes.type, job);
-    return job;
   }
 
   /**
@@ -540,6 +472,9 @@ class ModuleLoader {
 
     const result = FunctionPrototypeCall(translator, this, url, translateContext, parentURL);
     assert(result instanceof ModuleWrap, `The ${format} module returned is not a ModuleWrap`);
+    if (format === 'commonjs' || format === 'commonjs-sync' || format === 'require-commonjs') {
+      result.isCommonJS = true;
+    }
     return result;
   }
 
@@ -594,54 +529,72 @@ class ModuleLoader {
       return this.#translate(url, translateContext, parentURL);
     };
     if (isPromise(maybePromise)) {
-      return maybePromise.then(afterLoad);
+      return PromisePrototypeThen(maybePromise, afterLoad);
     }
     return afterLoad(maybePromise);
   }
 
   /**
-   * Load a module and translate it into a ModuleWrap, and create a ModuleJob from it.
+   * Given a resolved module request, obtain a ModuleJobBase from it - if it's already cached,
-   * This runs synchronously. If isForRequireInImportedCJS is true, the module should be linked
+   * return the cached ModuleJobBase. Otherwise, load its source and translate it into a ModuleWrap first.
-   * by the time this returns. Otherwise it may still have pending module requests.
+   * This runs synchronously. On any thread that is not an async loader hook worker thread,
-   * @param {string} url The URL that was resolved for this module.
+   * the module should be linked by the time this returns. Otherwise it may still have
-   * @param {ImportAttributes} importAttributes See {@link getModuleJobForImport}
+   * pending module requests.
-   * @param {number} phase Import phase.
+   * @param {string} parentURL See {@link getOrCreateModuleJob}
-   * @param {string} [parentURL] See {@link getModuleJobForImport}
+   * @param {{format: string, url: string}} resolveResult
-   * @param {string} [format] The format hint possibly returned by the `resolve` hook
+   * @param {ModuleRequest} request Module request.
-   * @param {boolean} isForRequireInImportedCJS Whether this module job is created for require()
+   * @param {ModuleRequestType} requestType Type of the module request.
-   *   in imported CJS.
    * @returns {ModuleJobBase} The (possibly pending) module job
    */
-  #createModuleJob(url, importAttributes, phase, parentURL, format, isForRequireInImportedCJS) {
+  #getOrCreateModuleJobAfterResolve(parentURL, resolveResult, request, requestType) {
-    const context = { format, importAttributes };
+    const { url, format } = resolveResult;
+    // TODO(joyeecheung): update the module requests to use importAttributes as property names.
+    const importAttributes = resolveResult.importAttributes ?? request.attributes;
+    let job = this.loadCache.get(url, importAttributes.type);
+
+    if (job !== undefined) {
+      if (requestType === kImportInRequiredESM) {
+        this.#checkCachedJobForRequireESM(request.specifier, url, parentURL, job);
+      }
+      job.ensurePhase(request.phase, requestType);
+      return job;
+    }
+
+    const context = { format, importAttributes, __proto__: null };
 
-    const isMain = parentURL === undefined;
     let moduleOrModulePromise;
-    if (isForRequireInImportedCJS) {
+    if (requestType === kRequireInImportedCJS) {
       moduleOrModulePromise = this.loadAndTranslateForRequireInImportedCJS(url, context, parentURL);
+    } else if (requestType === kImportInRequiredESM) {
+      moduleOrModulePromise = this.loadAndTranslateForImportInRequiredESM(url, context, parentURL, request);
     } else {
       moduleOrModulePromise = this.loadAndTranslate(url, context, parentURL);
     }
 
-    const inspectBrk = (
+    if (requestType === kImportInRequiredESM || requestType === kRequireInImportedCJS ||
-      isMain &&
+        !this.isForAsyncLoaderHookWorker) {
-      getOptionValue('--inspect-brk')
+      assert(moduleOrModulePromise instanceof ModuleWrap, `Expected ModuleWrap for loading ${url}`);
-    );
-
-    if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
-      process.send({ 'watch:import': [url] });
     }
 
-    const { ModuleJob } = require('internal/modules/esm/module_job');
+    if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
-    const job = new ModuleJob(
+      const type = requestType === kRequireInImportedCJS ? 'require' : 'import';
+      process.send({ [`watch:${type}`]: [url] });
+    }
+
+    const { ModuleJob, ModuleJobSync } = require('internal/modules/esm/module_job');
+    // TODO(joyeecheung): use ModuleJobSync for kRequireInImportedCJS too.
+    const ModuleJobCtor = (requestType === kImportInRequiredESM ? ModuleJobSync : ModuleJob);
+    const isMain = (parentURL === undefined);
+    const inspectBrk = (isMain && getOptionValue('--inspect-brk'));
+    job = new ModuleJobCtor(
       this,
       url,
       importAttributes,
       moduleOrModulePromise,
-      phase,
+      request.phase,
       isMain,
       inspectBrk,
-      isForRequireInImportedCJS,
+      requestType,
     );
 
     this.loadCache.set(url, importAttributes.type, job);
@@ -649,6 +602,34 @@ class ModuleLoader {
     return job;
   }
 
+  /**
+   * Get a (possibly not yet fully linked) module job from the cache, or create one and return its Promise.
+   * @param {string} [parentURL] The URL of the module where the module request is initiated.
+   *   It's undefined if it's from the root module.
+   * @param {ModuleRequest} request Module request.
+   * @param {ModuleRequestType} requestType Type of the module request.
+   * @returns {Promise<ModuleJobBase>|ModuleJobBase}
+   */
+  getOrCreateModuleJob(parentURL, request, requestType) {
+    let maybePromise;
+    if (requestType === kRequireInImportedCJS || requestType === kImportInRequiredESM) {
+      // In these two cases, resolution must be synchronous.
+      maybePromise = this.resolveSync(parentURL, request);
+      assert(!isPromise(maybePromise));
+    } else {
+      maybePromise = this.#resolve(parentURL, request);
+    }
+
+    const afterResolve = (resolveResult) => {
+      return this.#getOrCreateModuleJobAfterResolve(parentURL, resolveResult, request, requestType);
+    };
+
+    if (isPromise(maybePromise)) {
+      return PromisePrototypeThen(maybePromise, afterResolve);
+    }
+    return afterResolve(maybePromise);
+  }
+
   /**
    * This method is usually called indirectly as part of the loading processes.
    * Use directly with caution.
@@ -662,8 +643,16 @@ class ModuleLoader {
    */
   async import(specifier, parentURL, importAttributes, phase = kEvaluationPhase, isEntryPoint = false) {
     return onImport.tracePromise(async () => {
-      const moduleJob = await this.getModuleJobForImport(specifier, parentURL, importAttributes,
+      const request = { specifier, phase, attributes: importAttributes, __proto__: null };
-                                                         phase);
+      let moduleJob;
+      try {
+        moduleJob = await this.getOrCreateModuleJob(parentURL, request);
+      } catch (e) {
+        if (e?.code === 'ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED') {
+          return new Promise(() => {});
+        }
+        throw e;
+      }
       if (phase === kSourcePhase) {
         const module = await moduleJob.modulePromise;
         return module.getModuleSourceObject();
@@ -695,28 +684,20 @@ class ModuleLoader {
   /**
    * Resolve a module request to a URL identifying the location of the module. Handles customization hooks,
    * if any.
-   * @param {string|URL} specifier The module request of the module to be resolved. Typically, what's
-   *   requested by `import specifier`, `import(specifier)` or `import.meta.resolve(specifier)`.
    * @param {string} [parentURL] The URL of the module where the module request is initiated.
    *   It's undefined if it's from the root module.
-   * @param {ImportAttributes} importAttributes Attributes from the import statement or expression.
+   * @param {ModuleRequest} request Module request.
-   * @returns {Promise<{format: string, url: string}>}
+   * @returns {Promise<{format: string, url: string}>|{format: string, url: string}}
    */
-  resolve(specifier, parentURL, importAttributes) {
+  #resolve(parentURL, request) {
-    specifier = `${specifier}`;
+    if (this.isForAsyncLoaderHookWorker) {
-    if (syncResolveHooks.length) {
+      const specifier = `${request.specifier}`;
-      // Has module.registerHooks() hooks, use the synchronous variant that can handle both hooks.
+      const importAttributes = request.attributes ?? kEmptyObject;
-      return this.resolveSync(specifier, parentURL, importAttributes);
+      // TODO(joyeecheung): invoke the synchronous hooks in the default step on loader thread.
-    }
-    if (this.#asyncLoaderHooks) {  // Only has module.register hooks.
       return this.#asyncLoaderHooks.resolve(specifier, parentURL, importAttributes);
     }
-    return this.#cachedDefaultResolve(specifier, {
+
-      __proto__: null,
+    return this.resolveSync(parentURL, request);
-      conditions: this.#defaultConditions,
-      parentURL,
-      importAttributes,
-    });
   }
 
   /**
@@ -739,25 +720,6 @@ class ModuleLoader {
     return result;
   }
 
-  /**
-   * Either return a cached resolution, or perform the synchronous resolution, and
-   * cache the result.
-   * @param {string} specifier See {@link resolve}.
-   * @param {string} [parentURL] See {@link resolve}.
-   * @param {ImportAttributes} importAttributes See {@link resolve}.
-   * @returns {{ format: string, url: string }}
-   */
-  #cachedResolveSync(specifier, parentURL, importAttributes) {
-    const requestKey = this.#resolveCache.serializeKey(specifier, importAttributes);
-    const cachedResult = this.#resolveCache.get(requestKey, parentURL);
-    if (cachedResult != null) {
-      return cachedResult;
-    }
-    const result = this.resolveSync(specifier, parentURL, importAttributes);
-    this.#resolveCache.set(requestKey, parentURL, result);
-    return result;
-  }
-
   /**
    * This is the default resolve step for module.registerHooks(), which incorporates asynchronous hooks
    * from module.register() which are run in a blocking fashion for it to be synchronous.
@@ -767,7 +729,7 @@ class ModuleLoader {
    * @returns {{ format: string, url: string }}
    */
   #resolveAndMaybeBlockOnLoaderThread(specifier, context) {
-    if (this.#asyncLoaderHooks) {
+    if (this.#asyncLoaderHooks?.resolveSync) {
       return this.#asyncLoaderHooks.resolveSync(specifier, context.parentURL, context.importAttributes);
     }
     return this.#cachedDefaultResolve(specifier, context);
@@ -779,46 +741,43 @@ class ModuleLoader {
    * from the loader thread for this to be synchronous.
    * This is here to support `import.meta.resolve()`, `require()` in imported CJS, and
    * `module.registerHooks()` hooks.
-   *
-   * TODO(joyeecheung): consolidate the cache behavior and use this in require(esm).
-   * @param {string|URL} specifier See {@link resolve}.
    * @param {string} [parentURL] See {@link resolve}.
-   * @param {ImportAttributes} [importAttributes] See {@link resolve}.
+   * @param {ModuleRequest} request See {@link resolve}.
    * @returns {{ format: string, url: string }}
    */
-  resolveSync(specifier, parentURL, importAttributes = { __proto__: null }) {
+  resolveSync(parentURL, request) {
-    specifier = `${specifier}`;
+    const specifier = `${request.specifier}`;
+    const importAttributes = request.attributes ?? kEmptyObject;
+
     if (syncResolveHooks.length) {
       // Has module.registerHooks() hooks, chain the asynchronous hooks in the default step.
       return resolveWithSyncHooks(specifier, parentURL, importAttributes, this.#defaultConditions,
                                   this.#resolveAndMaybeBlockOnLoaderThread.bind(this));
     }
-    return this.#resolveAndMaybeBlockOnLoaderThread(specifier, {
+    const context = {
-      __proto__: null,
+      ...request,
       conditions: this.#defaultConditions,
       parentURL,
       importAttributes,
-    });
+      __proto__: null,
+    };
+    return this.#resolveAndMaybeBlockOnLoaderThread(specifier, context);
   }
 
   /**
    * Provide source that is understood by one of Node's translators. Handles customization hooks,
    * if any.
+   * @typedef { {format: ModuleFormat, source: ModuleSource }} LoadResult
    * @param {string} url The URL of the module to be loaded.
    * @param {object} context Metadata about the module
-   * @returns {Promise<{ format: ModuleFormat, source: ModuleSource }> | { format: ModuleFormat, source: ModuleSource }}
+   * @returns {Promise<LoadResult> | LoadResult}}
    */
   load(url, context) {
-    if (syncLoadHooks.length) {
+    if (this.isForAsyncLoaderHookWorker) {
-      // Has module.registerHooks() hooks, use the synchronous variant that can handle both hooks.
+      // TODO(joyeecheung): invoke the synchronous hooks in the default step on loader thread.
-      return this.#loadSync(url, context);
-    }
-    if (this.#asyncLoaderHooks) {
       return this.#asyncLoaderHooks.load(url, context);
     }
-
+    return this.#loadSync(url, context);
-    defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync;
-    return defaultLoadSync(url, context);
   }
 
   /**
@@ -829,7 +788,7 @@ class ModuleLoader {
    * @returns {{ format: ModuleFormat, source: ModuleSource }}
    */
   #loadAndMaybeBlockOnLoaderThread(url, context) {
-    if (this.#asyncLoaderHooks) {
+    if (this.#asyncLoaderHooks?.loadSync) {
       return this.#asyncLoaderHooks.loadSync(url, context);
     }
     defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync;
@@ -841,8 +800,6 @@ class ModuleLoader {
    * from module.register(), this blocks on the loader thread for it to return synchronously.
    *
    * This is here to support `require()` in imported CJS and `module.registerHooks()` hooks.
-   *
-   * TODO(joyeecheung): consolidate the cache behavior and use this in require(esm).
    * @param {string} url See {@link load}
    * @param {object} [context] See {@link load}
    * @returns {{ format: ModuleFormat, source: ModuleSource }}

lib/internal/modules/esm/module_job.js

@@ -3,6 +3,7 @@
 const {
   Array,
   ArrayPrototypeJoin,
+  ArrayPrototypePush,
   ArrayPrototypeSome,
   FunctionPrototype,
   ObjectSetPrototypeOf,
@@ -36,7 +37,10 @@ const {
     entry_point_module_private_symbol,
   },
 } = internalBinding('util');
-const { decorateErrorStack, kEmptyObject } = require('internal/util');
+/**
+ * @typedef {import('./utils.js').ModuleRequestType} ModuleRequestType
+ */
+const { decorateErrorStack } = require('internal/util');
 const { isPromise } = require('internal/util/types');
 const {
   getSourceMapsSupport,
@@ -106,8 +110,9 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => {
 };
 
 class ModuleJobBase {
-  constructor(url, importAttributes, phase, isMain, inspectBrk) {
+  constructor(loader, url, importAttributes, phase, isMain, inspectBrk) {
     assert(typeof phase === 'number');
+    this.loader = loader;
     this.importAttributes = importAttributes;
     this.phase = phase;
     this.isMain = isMain;
@@ -115,13 +120,65 @@ class ModuleJobBase {
 
     this.url = url;
   }
+
+  /**
+   * Synchronously link the module and its dependencies.
+   * @param {ModuleRequestType} requestType Type of the module request.
+   * @returns {ModuleJobBase[]}
+   */
+  syncLink(requestType) {
+    // Store itself into the cache first before linking in case there are circular
+    // references in the linking.
+    this.loader.loadCache.set(this.url, this.type, this);
+    const moduleRequests = this.module.getModuleRequests();
+    // Modules should be aligned with the moduleRequests array in order.
+    const modules = Array(moduleRequests.length);
+    const evaluationDepJobs = [];
+    this.commonJsDeps = Array(moduleRequests.length);
+    try {
+      for (let idx = 0; idx < moduleRequests.length; idx++) {
+        const request = moduleRequests[idx];
+        // TODO(joyeecheung): split this into two iterators, one for resolving and one for loading so
+        // that hooks can pre-fetch sources off-thread.
+        const job = this.loader.getOrCreateModuleJob(this.url, request, requestType);
+        debug(`ModuleJobBase.syncLink() ${this.url} -> ${request.specifier}`, job);
+        assert(!isPromise(job));
+        assert(job.module instanceof ModuleWrap);
+        if (request.phase === kEvaluationPhase) {
+          ArrayPrototypePush(evaluationDepJobs, job);
+        }
+        modules[idx] = job.module;
+        this.commonJsDeps[idx] = job.module.isCommonJS;
+      }
+      this.module.link(modules);
+    } finally {
+      // Restore it - if it succeeds, we'll reset in the caller; Otherwise it's
+      // not cached and if the error is caught, subsequent attempt would still fail.
+      this.loader.loadCache.delete(this.url, this.type);
+    }
+
+    return evaluationDepJobs;
+  }
+
+  /**
+   * Ensure that this ModuleJob is moving towards the required phase
+   * (does not necessarily mean it is ready at that phase - run does that)
+   * @param {number} phase
+   */
+  ensurePhase(phase, requestType) {
+    if (this.phase < phase) {
+      this.phase = phase;
+      this.linked = this.link(requestType);
+      if (isPromise(this.linked)) {
+        PromisePrototypeThen(this.linked, undefined, noop);
+      }
+    }
+  }
 }
 
 /* A ModuleJob tracks the loading of a single Module, and the ModuleJobs of
  * its dependencies, over time. */
 class ModuleJob extends ModuleJobBase {
-  #loader = null;
-
   /**
    * @param {ModuleLoader} loader The ESM loader.
    * @param {string} url URL of the module to be wrapped in ModuleJob.
@@ -131,12 +188,11 @@ class ModuleJob extends ModuleJobBase {
    * @param {boolean} isMain Whether the module is the entry point.
    * @param {boolean} inspectBrk Whether this module should be evaluated with the
    *   first line paused in the debugger (because --inspect-brk is passed).
-   * @param {boolean} isForRequireInImportedCJS Whether this is created for require() in imported CJS.
+   * @param {ModuleRequestType} requestType Type of the module request.
    */
   constructor(loader, url, importAttributes = { __proto__: null }, moduleOrModulePromise,
-              phase = kEvaluationPhase, isMain, inspectBrk, isForRequireInImportedCJS = false) {
+              phase = kEvaluationPhase, isMain, inspectBrk, requestType) {
-    super(url, importAttributes, phase, isMain, inspectBrk);
+    super(loader, url, importAttributes, phase, isMain, inspectBrk);
-    this.#loader = loader;
 
     // Expose the promise to the ModuleWrap directly for linking below.
     if (isPromise(moduleOrModulePromise)) {
@@ -148,10 +204,12 @@ class ModuleJob extends ModuleJobBase {
 
     if (this.phase === kEvaluationPhase) {
       // Promise for the list of all dependencyJobs.
-      this.linked = this.#link();
+      this.linked = this.link(requestType);
       // This promise is awaited later anyway, so silence
       // 'unhandled rejection' warnings.
-      PromisePrototypeThen(this.linked, undefined, noop);
+      if (isPromise(this.linked)) {
+        PromisePrototypeThen(this.linked, undefined, noop);
+      }
     }
 
     // instantiated == deep dependency jobs wrappers are instantiated,
@@ -160,79 +218,69 @@ class ModuleJob extends ModuleJobBase {
   }
 
   /**
-   * Ensure that this ModuleJob is moving towards the required phase
+   * @param {ModuleRequestType} requestType Type of the module request.
-   * (does not necessarily mean it is ready at that phase - run does that)
+   * @returns {ModuleJobBase[]|Promise<ModuleJobBase[]>}
-   * @param {number} phase
    */
-  ensurePhase(phase) {
+  link(requestType) {
-    if (this.phase < phase) {
+    if (this.loader.isForAsyncLoaderHookWorker) {
-      this.phase = phase;
+      return this.#asyncLink(requestType);
-      this.linked = this.#link();
-      PromisePrototypeThen(this.linked, undefined, noop);
     }
+    return this.syncLink(requestType);
   }
 
   /**
-   * Iterates the module requests and links with the loader.
+   * @param {ModuleRequestType} requestType Type of the module request.
-   * @returns {Promise<ModuleJob[]>} Dependency module jobs.
+   * @returns {Promise<ModuleJobBase[]>}
    */
-  async #link() {
+  async #asyncLink(requestType) {
+    assert(this.loader.isForAsyncLoaderHookWorker);
     this.module = await this.modulePromise;
     assert(this.module instanceof ModuleWrap);
-
     const moduleRequests = this.module.getModuleRequests();
-    // Explicitly keeping track of dependency jobs is needed in order
-    // to flatten out the dependency graph below in `_instantiate()`,
-    // so that circular dependencies can't cause a deadlock by two of
-    // these `link` callbacks depending on each other.
     // Create an ArrayLike to avoid calling into userspace with `.then`
     // when returned from the async function.
-    const evaluationDepJobs = Array(moduleRequests.length);
-    ObjectSetPrototypeOf(evaluationDepJobs, null);
-
     // Modules should be aligned with the moduleRequests array in order.
     const modulePromises = Array(moduleRequests.length);
-    // Track each loop for whether it is an evaluation phase or source phase request.
+    const evaluationDepJobs = [];
-    let isEvaluation;
+    this.commonJsDeps = Array(moduleRequests.length);
-    // Iterate with index to avoid calling into userspace with `Symbol.iterator`.
+    for (let idx = 0; idx < moduleRequests.length; idx++) {
-    for (
+      const request = moduleRequests[idx];
-      let idx = 0, eidx = 0;
+      // Explicitly keeping track of dependency jobs is needed in order
-      // Use the let-scoped eidx value to update the executionDepJobs length at the end of the loop.
+      // to flatten out the dependency graph below in `asyncInstantiate()`,
-      idx < moduleRequests.length || (evaluationDepJobs.length = eidx, false);
+      // so that circular dependencies can't cause a deadlock by two of
-      idx++, eidx += isEvaluation
+      // these `link` callbacks depending on each other.
-    ) {
+      // TODO(joyeecheung): split this into two iterators, one for resolving and one for loading so
-      const { specifier, phase, attributes } = moduleRequests[idx];
+      // that hooks can pre-fetch sources off-thread.
-      isEvaluation = phase === kEvaluationPhase;
+      const dependencyJobPromise = this.loader.getOrCreateModuleJob(this.url, request, requestType);
-      // TODO(joyeecheung): resolve all requests first, then load them in another
-      // loop so that hooks can pre-fetch sources off-thread.
-      const dependencyJobPromise = this.#loader.getModuleJobForImport(
-        specifier, this.url, attributes, phase,
-      );
       const modulePromise = PromisePrototypeThen(dependencyJobPromise, (job) => {
-        debug(`async link() ${this.url} -> ${specifier}`, job);
+        debug(`ModuleJob.asyncLink() ${this.url} -> ${request.specifier}`, job);
-        if (phase === kEvaluationPhase) {
+        if (request.phase === kEvaluationPhase) {
-          evaluationDepJobs[eidx] = job;
+          ArrayPrototypePush(evaluationDepJobs, job);
         }
         return job.modulePromise;
       });
       modulePromises[idx] = modulePromise;
     }
-
     const modules = await SafePromiseAllReturnArrayLike(modulePromises);
-    this.module.link(modules);
+    for (let idx = 0; idx < moduleRequests.length; idx++) {
+      this.commonJsDeps[idx] = modules[idx].isCommonJS;
+    }
 
+    this.module.link(modules);
     return evaluationDepJobs;
   }
 
   #instantiate() {
     if (this.instantiated === undefined) {
-      this.instantiated = this.#_instantiate();
+      this.instantiated = this.#asyncInstantiate();
     }
     return this.instantiated;
   }
 
-  async #_instantiate() {
+  async #asyncInstantiate() {
     const jobsInGraph = new SafeSet();
+    // TODO(joyeecheung): if it's not on the async loader thread, consider this already
+    // linked.
     const addJobsToDependencyGraph = async (moduleJob) => {
       debug(`async addJobsToDependencyGraph() ${this.url}`, moduleJob);
 
@@ -240,7 +288,7 @@ class ModuleJob extends ModuleJobBase {
         return;
       }
       jobsInGraph.add(moduleJob);
-      const dependencyJobs = await moduleJob.linked;
+      const dependencyJobs = isPromise(moduleJob.linked) ? await moduleJob.linked : moduleJob.linked;
       return SafePromiseAllReturnVoid(dependencyJobs, addJobsToDependencyGraph);
     };
     await addJobsToDependencyGraph(this);
@@ -263,31 +311,19 @@ class ModuleJob extends ModuleJobBase {
           StringPrototypeIncludes(e.message,
                                   ' does not provide an export named')) {
         const splitStack = StringPrototypeSplit(e.stack, '\n', 2);
-        const parentFileUrl = RegExpPrototypeSymbolReplace(
-          /:\d+$/,
-          splitStack[0],
-          '',
-        );
         const { 1: childSpecifier, 2: name } = RegExpPrototypeExec(
           /module '(.*)' does not provide an export named '(.+)'/,
           e.message);
-        const { url: childFileURL } = await this.#loader.resolve(
+        const moduleRequests = this.module.getModuleRequests();
-          childSpecifier,
+        let isCommonJS = false;
-          parentFileUrl,
+        for (let i = 0; i < moduleRequests.length; ++i) {
-          kEmptyObject,
+          if (moduleRequests[i].specifier === childSpecifier) {
-        );
+            isCommonJS = this.commonJsDeps[i];
-        let format;
+            break;
-        try {
+          }
-          // This might throw for non-CommonJS modules because we aren't passing
-          // in the import attributes and some formats require them; but we only
-          // care about CommonJS for the purposes of this error message.
-          ({ format } =
-            await this.#loader.load(childFileURL));
-        } catch {
-          // Continue regardless of error.
         }
 
-        if (format === 'commonjs') {
+        if (isCommonJS) {
           const importStatement = splitStack[1];
           // TODO(@ctavan): The original error stack only provides the single
           // line which causes the error. For multi-line import statements we
@@ -394,8 +430,6 @@ class ModuleJob extends ModuleJobBase {
  * TODO(joyeecheung): consolidate this with the isForRequireInImportedCJS variant of ModuleJob.
  */
 class ModuleJobSync extends ModuleJobBase {
-  #loader = null;
-
   /**
    * @param {ModuleLoader} loader The ESM loader.
    * @param {string} url URL of the module to be wrapped in ModuleJob.
@@ -407,57 +441,26 @@ class ModuleJobSync extends ModuleJobBase {
    *   first line paused in the debugger (because --inspect-brk is passed).
    */
   constructor(loader, url, importAttributes, moduleWrap, phase = kEvaluationPhase, isMain,
-              inspectBrk) {
+              inspectBrk, requestType) {
-    super(url, importAttributes, phase, isMain, inspectBrk, true);
+    super(loader, url, importAttributes, phase, isMain, inspectBrk);
 
-    this.#loader = loader;
     this.module = moduleWrap;
 
     assert(this.module instanceof ModuleWrap);
     this.linked = undefined;
     this.type = importAttributes.type;
     if (phase === kEvaluationPhase) {
-      this.#link();
+      this.linked = this.link(requestType);
     }
   }
 
   /**
-   * Ensure that this ModuleJob is at the required phase
+   * @param {ModuleRequestType} requestType Type of the module request.
-   * @param {number} phase
+   * @returns {ModuleJobBase[]}
    */
-  ensurePhase(phase) {
+  link(requestType) {
-    if (this.phase < phase) {
+    // Synchronous linking is always used for ModuleJobSync.
-      this.phase = phase;
+    return this.syncLink(requestType);
-      this.#link();
-    }
-  }
-
-  #link() {
-    // Store itself into the cache first before linking in case there are circular
-    // references in the linking.
-    this.#loader.loadCache.set(this.url, this.type, this);
-    try {
-      const moduleRequests = this.module.getModuleRequests();
-      // Modules should be aligned with the moduleRequests array in order.
-      const modules = Array(moduleRequests.length);
-      const evaluationDepJobs = Array(moduleRequests.length);
-      let j = 0;
-      for (let i = 0; i < moduleRequests.length; ++i) {
-        const { specifier, attributes, phase } = moduleRequests[i];
-        const job = this.#loader.getModuleJobForRequire(specifier, this.url, attributes, phase);
-        modules[i] = job.module;
-        if (phase === kEvaluationPhase) {
-          evaluationDepJobs[j++] = job;
-        }
-      }
-      evaluationDepJobs.length = j;
-      this.module.link(modules);
-      this.linked = evaluationDepJobs;
-    } finally {
-      // Restore it - if it succeeds, we'll reset in the caller; Otherwise it's
-      // not cached and if the error is caught, subsequent attempt would still fail.
-      this.#loader.loadCache.delete(this.url, this.type);
-    }
   }
 
   get modulePromise() {

lib/internal/modules/esm/translators.js

@@ -57,7 +57,7 @@ const {
 } = require('internal/errors').codes;
 const { maybeCacheSourceMap } = require('internal/source_map/source_map_cache');
 const moduleWrap = internalBinding('module_wrap');
-const { ModuleWrap } = moduleWrap;
+const { ModuleWrap, kEvaluationPhase } = moduleWrap;
 
 // Lazy-loading to avoid circular dependencies.
 let getSourceSync;
@@ -112,6 +112,7 @@ translators.set('module', function moduleStrategy(url, translateContext, parentU
   return module;
 });
 
+const { requestTypes: { kRequireInImportedCJS } } = require('internal/modules/esm/utils');
 /**
  * Loads a CommonJS module via the ESM Loader sync CommonJS translator.
  * This translator creates its own version of the `require` function passed into CommonJS modules.
@@ -131,7 +132,6 @@ function loadCJSModule(module, source, url, filename, isMain) {
   if (sourceMapURL) {
     maybeCacheSourceMap(url, source, module, false, sourceURL, sourceMapURL);
   }
-
   const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
   const __dirname = dirname(filename);
   // eslint-disable-next-line func-name-matching,func-style
@@ -154,7 +154,8 @@ function loadCJSModule(module, source, url, filename, isMain) {
 
     // FIXME(node:59666) Currently, the ESM loader re-invents require() here for imported CJS and this
     // requires a separate cache to be populated as well as introducing several quirks. This is not ideal.
-    const job = cascadedLoader.getModuleJobForRequireInImportedCJS(specifier, url, importAttributes);
+    const request = { specifier, attributes: importAttributes, phase: kEvaluationPhase, __proto__: null };
+    const job = cascadedLoader.getOrCreateModuleJob(url, request, kRequireInImportedCJS);
     job.runSync();
     let mod = cjsCache.get(job.url);
     assert(job.module, `Imported CJS module ${url} failed to load module ${job.url} using require() due to race condition`);
@@ -185,7 +186,9 @@ function loadCJSModule(module, source, url, filename, isMain) {
         specifier = `${pathToFileURL(path)}`;
       }
     }
-    const { url: resolvedURL } = cascadedLoader.resolveSync(specifier, url, kEmptyObject);
+
+    const request = { specifier, __proto__: null, attributes: kEmptyObject };
+    const { url: resolvedURL } = cascadedLoader.resolveSync(url, request);
     return urlToFilename(resolvedURL);
   });
   setOwnProperty(requireFn, 'main', process.mainModule);

lib/internal/modules/esm/utils.js

@@ -5,6 +5,7 @@ const {
   ObjectFreeze,
   SafeSet,
   SafeWeakMap,
+  Symbol,
 } = primordials;
 
 const {
@@ -332,6 +333,16 @@ function compileSourceTextModule(url, source, cascadedLoader, context = kEmptyOb
   return wrap;
 }
 
+
+const kImportInImportedESM = Symbol('kImportInImportedESM');
+const kImportInRequiredESM = Symbol('kImportInRequiredESM');
+const kRequireInImportedCJS = Symbol('kRequireInImportedCJS');
+
+/**
+ * @typedef {kImportInImportedESM | kImportInRequiredESM | kRequireInImportedCJS} ModuleRequestType
+ */
+const requestTypes = { kImportInImportedESM, kImportInRequiredESM, kRequireInImportedCJS };
+
 module.exports = {
   registerModule,
   initializeESM,
@@ -339,4 +350,5 @@ module.exports = {
   getConditionsSet,
   shouldSpawnLoaderHookWorker,
   compileSourceTextModule,
+  requestTypes,
 };

lib/internal/test_runner/mock/mock.js

@@ -646,9 +646,8 @@ class MockTracker {
     // If the caller is already a file URL, use it as is. Otherwise, convert it.
     const hasFileProtocol = StringPrototypeStartsWith(filename, 'file://');
     const caller = hasFileProtocol ? filename : pathToFileURL(filename).href;
-    const { format, url } = sharedState.moduleLoader.resolveSync(
+    const request = { __proto__: null, specifier: mockSpecifier, attributes: kEmptyObject };
-      mockSpecifier, caller, kEmptyObject,
+    const { format, url } = sharedState.moduleLoader.resolveSync(caller, request);
-    );
     debug('module mock, url = "%s", format = "%s", caller = "%s"', url, format, caller);
     if (format) { // Format is not yet known for ambiguous files when detection is enabled.
       validateOneOf(format, 'format', kSupportedFormats);

test/es-module/test-esm-loader-hooks.mjs

@@ -119,7 +119,7 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => {
       assert.strictEqual(signal, null);
     });
 
-    it('import.meta.resolve of a never-settling resolve', async () => {
+    it('import.meta.resolve of a never-settling resolve should throw', async () => {
       const { code, signal, stdout, stderr } = await spawnPromisified(execPath, [
         '--no-warnings',
         '--experimental-loader',
@@ -129,7 +129,7 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => {
 
       assert.strictEqual(stderr, '');
       assert.match(stdout, /^should be output\r?\n$/);
-      assert.strictEqual(code, 13);
+      assert.strictEqual(code, 0);
       assert.strictEqual(signal, null);
     });
   });
@@ -668,13 +668,17 @@ describe('Loader hooks', { concurrency: !process.env.TEST_PARALLEL }, () => {
         '--eval',
         `
         import {register} from 'node:module';
-        register('data:text/javascript,export function initialize(){return new Promise(()=>{})}');
+        try {
+          register('data:text/javascript,export function initialize(){return new Promise(()=>{})}');
+        } catch (e) {
+          console.log('caught', e.code);
+        }
         `,
       ]);
 
       assert.strictEqual(stderr, '');
-      assert.strictEqual(stdout, '');
+      assert.strictEqual(stdout.trim(), 'caught ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED');
-      assert.strictEqual(code, 13);
+      assert.strictEqual(code, 0);
       assert.strictEqual(signal, null);
     });
 

test/es-module/test-esm-loader-modulemap.js

@@ -17,12 +17,9 @@ const stubJsModule = createDynamicModule([], ['default'], jsModuleDataUrl);
 const stubJsonModule = createDynamicModule([], ['default'], jsonModuleDataUrl);
 
 const loader = createModuleLoader();
-const jsModuleJob = new ModuleJob(loader, stubJsModule.module, undefined,
+const jsModuleJob = new ModuleJob(loader, jsModuleDataUrl, {}, stubJsModule.module);
-                                  () => new Promise(() => {}));
+const jsonModuleJob = new ModuleJob(loader, jsonModuleDataUrl,
-const jsonModuleJob = new ModuleJob(loader, stubJsonModule.module,
+                                    { type: 'json' }, stubJsonModule.module);
-                                    { type: 'json' },
-                                    () => new Promise(() => {}));
-
 
 // LoadCache.set and LoadCache.get store and retrieve module jobs for a
 // specified url/type tuple; LoadCache.has correctly reports whether such jobs

test/fixtures/es-module-loaders/never-settling-resolve-step/import.meta.never-resolve.mjs

@@ -1,5 +1,8 @@
+import assert from 'node:assert';
 console.log('should be output');
 
-import.meta.resolve('never-settle-resolve');
+assert.throws(() => {
-
+  import.meta.resolve('never-settle-resolve');
-console.log('should not be output');
+}, {
+  code: 'ERR_ASYNC_LOADER_REQUEST_NEVER_SETTLED'
+});

test/fixtures/module-hooks/logger-async-hooks.mjs

@@ -0,0 +1,19 @@
+import fs from 'node:fs';
+
+export async function resolve(specifier, context, nextResolve) {
+  fs.writeSync(1, `resolve ${specifier} from ${context.parentURL}\n`);
+  const result = await nextResolve(specifier, context);
+  result.shortCircuit = true;
+  return result;
+}
+
+export async function load(url, context, nextLoad) {
+  fs.writeSync(1, `load ${url}\n`);
+  const result = await nextLoad(url, context);
+  result.shortCircuit = true;
+  // Handle the async loader hook quirk where source can be nullish for CommonJS.
+  // If this returns that nullish value verbatim, the `require` in imported
+  // CJS won't get the hook invoked.
+  result.source ??= fs.readFileSync(new URL(url), 'utf8');
+  return result;
+}

test/fixtures/module-hooks/register-logger-async-hooks.mjs

@@ -0,0 +1,2 @@
+import { register } from 'node:module';
+register('./logger-async-hooks.mjs', import.meta.url);

test/fixtures/module-hooks/require-esm/cjs.cjs

@@ -0,0 +1 @@
+exports.cjsValue = require('./inner.cjs');

test/fixtures/module-hooks/require-esm/esm.mjs

@@ -0,0 +1,2 @@
+export { esmValue } from './inner.mjs'
+export { cjsValue } from './cjs.cjs'

test/fixtures/module-hooks/require-esm/inner.cjs

@@ -0,0 +1 @@
+module.exports = 'commonjs';

test/fixtures/module-hooks/require-esm/inner.mjs

@@ -0,0 +1 @@
+export const esmValue = 'esm';

test/fixtures/module-hooks/require-esm/main.cjs

@@ -0,0 +1,4 @@
+const { esmValue, cjsValue } = require('./esm.mjs');
+console.log('esmValue in main.cjs:', esmValue);
+console.log('cjsValue in main.cjs:', cjsValue);
+module.exports = { esmValue, cjsValue };

test/fixtures/module-hooks/require-esm/main.mjs

@@ -0,0 +1,3 @@
+import { esmValue, cjsValue } from './main.cjs';
+console.log('esmValue in main.mjs:', esmValue);
+console.log('cjsValue in main.mjs:', cjsValue);

test/module-hooks/test-module-hooks-require-esm.js

@@ -0,0 +1,72 @@
+'use strict';
+
+// This tests that the async loader hooks can be invoked for require(esm).
+
+require('../common');
+const common = require('../common');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const fixtures = require('../common/fixtures');
+const assert = require('assert');
+
+function assertSubGraph(output) {
+  // FIXME(node:59666): the async resolve hook invoked for require() in imported CJS only get the URL,
+  // not the specifier. This may be fixable if we re-implement the async loader hooks from within
+  // the synchronous loader hooks and use the original require() implementation.
+  assert.match(output, /resolve .+esm\.mjs from file:.+main\.cjs/);
+  assert.match(output, /load file:.+esm\.mjs/);
+
+  assert.match(output, /resolve \.\/inner\.mjs from file:.+esm\.mjs/);
+  assert.match(output, /load file:.+inner\.mjs/);
+
+  assert.match(output, /resolve \.\/cjs\.cjs from file:.+esm\.mjs/);
+  assert.match(output, /load file:.+cjs\.cjs/);
+
+  // FIXME(node:59666): see above.
+  assert.match(output, /resolve .+inner\.cjs from file:.+cjs\.cjs/);
+  assert.match(output, /load file:.+inner\.cjs/);
+
+  assert.match(output, /esmValue in main\.cjs: esm/);
+  assert.match(output, /cjsValue in main\.cjs: commonjs/);
+}
+
+spawnSyncAndAssert(process.execPath, [
+  '--import',
+  fixtures.fileURL('module-hooks', 'register-logger-async-hooks.mjs'),
+  fixtures.path('module-hooks', 'require-esm', 'main.cjs'),
+], {
+  stdout: common.mustCall((output) => {
+    // The graph is:
+    //   main.cjs
+    //     -> esm.mjs
+    //       -> inner.mjs
+    //       -> cjs.cjs
+    //         -> inner.cjs
+    assert.match(output, /resolve .+main\.cjs from undefined/);
+    assert.match(output, /load file:.+main\.cjs/);
+
+    assertSubGraph(output);
+  }),
+});
+
+spawnSyncAndAssert(process.execPath, [
+  '--import',
+  fixtures.fileURL('module-hooks', 'register-logger-async-hooks.mjs'),
+  fixtures.path('module-hooks', 'require-esm', 'main.mjs'),
+], {
+  stdout: common.mustCall((output) => {
+    // The graph is:
+    //   main.mjs
+    //     -> main.cjs
+    //       -> esm.mjs
+    //         -> inner.mjs
+    //         -> cjs.cjs
+    //           -> inner.cjs
+    assert.match(output, /resolve .+main\.mjs from undefined/);
+    assert.match(output, /load file:.+main\.mjs/);
+
+    assert.match(output, /resolve .+main\.cjs from file:.+main\.mjs/);
+    assert.match(output, /load file:.+main\.cjs/);
+
+    assertSubGraph(output);
+  }),
+});