module: handle null source from async loader hooks in sync hooks

This relaxes the validation in sync hooks so that it accepts
the quirky nullish source returned by the default step of the
async loader when the module being loaded is CommonJS.
When there are no customization hooks registered, a saner
synchronous default load step is used to use a property
instead of a reset nullish source to signify that the module
should go through the CJS monkey patching routes and reduce
excessive reloading from disk.

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

lib/internal/modules/cjs/loader.js

@@ -177,6 +177,7 @@ const {
   registerHooks,
   resolveHooks,
   resolveWithHooks,
+  validateLoadStrict,
 } = require('internal/modules/customization_hooks');
 const { stripTypeScriptModuleTypes } = require('internal/modules/typescript');
 const packageJsonReader = require('internal/modules/package_json_reader');
@@ -1175,7 +1176,7 @@ function loadBuiltinWithHooks(id, url, format) {
     url ??= `node:${id}`;
     // TODO(joyeecheung): do we really want to invoke the load hook for the builtins?
     const loadResult = loadWithHooks(url, format || 'builtin', /* importAttributes */ undefined,
-                                     getCjsConditionsArray(), getDefaultLoad(url, id));
+                                     getCjsConditionsArray(), getDefaultLoad(url, id), validateLoadStrict);
     if (loadResult.format && loadResult.format !== 'builtin') {
       return undefined;  // Format has been overridden, return undefined for the caller to continue loading.
     }
@@ -1791,10 +1792,9 @@ function loadSource(mod, filename, formatFromNode) {
     mod[kURL] = convertCJSFilenameToURL(filename);
   }
 
+  const defaultLoad = getDefaultLoad(mod[kURL], filename);
   const loadResult = loadWithHooks(mod[kURL], mod[kFormat], /* importAttributes */ undefined,
-                                   getCjsConditionsArray(),
-                                   getDefaultLoad(mod[kURL], filename));
-
+                                   getCjsConditionsArray(), defaultLoad, validateLoadStrict);
   // Reset the module properties with load hook results.
   if (loadResult.format !== undefined) {
     mod[kFormat] = loadResult.format;

lib/internal/modules/customization_hooks.js

@@ -262,13 +262,25 @@ function validateResolve(specifier, context, result) {
  */
 
 /**
- * Validate the result returned by a chain of resolve hook.
+ * Validate the result returned by a chain of load hook.
  * @param {string} url URL passed into the hooks.
  * @param {ModuleLoadContext} context Context passed into the hooks.
  * @param {ModuleLoadResult} result Result produced by load hooks.
  * @returns {ModuleLoadResult}
  */
-function validateLoad(url, context, result) {
+function validateLoadStrict(url, context, result) {
+  validateSourceStrict(url, context, result);
+  validateFormat(url, context, result);
+  return result;
+}
+
+function validateLoadSloppy(url, context, result) {
+  validateSourcePermissive(url, context, result);
+  validateFormat(url, context, result);
+  return result;
+}
+
+function validateSourceStrict(url, context, result) {
   const { source, format } = result;
   // To align with module.register(), the load hooks are still invoked for
   // the builtins even though the default load step only provides null as source,
@@ -276,7 +288,8 @@ function validateLoad(url, context, result) {
   if (!StringPrototypeStartsWith(url, 'node:') &&
     typeof result.source !== 'string' &&
       !isAnyArrayBuffer(source) &&
-      !isArrayBufferView(source)) {
+      !isArrayBufferView(source) &&
+      format !== 'addon') {
     throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
       'a string, an ArrayBuffer, or a TypedArray',
       'load',
@@ -284,7 +297,21 @@ function validateLoad(url, context, result) {
       source,
     );
   }
+}
 
+function validateSourcePermissive(url, context, result) {
+  const { source, format } = result;
+  if (format === 'commonjs' && source == null) {
+    // Accommodate the quirk in defaultLoad used by asynchronous loader hooks
+    // which sets source to null for commonjs.
+    // See: https://github.com/nodejs/node/issues/57327#issuecomment-2701382020
+    return;
+  }
+  validateSourceStrict(url, context, result);
+}
+
+function validateFormat(url, context, result) {
+  const { format } = result;
   if (typeof format !== 'string' && format !== undefined) {
     throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
       'a string',
@@ -293,12 +320,6 @@ function validateLoad(url, context, result) {
       format,
     );
   }
-
-  return {
-    __proto__: null,
-    format,
-    source,
-  };
 }
 
 class ModuleResolveContext {
@@ -338,9 +359,10 @@ let decoder;
  * @param {ImportAttributes|undefined} importAttributes
  * @param {string[]} conditions
  * @param {(url: string, context: ModuleLoadContext) => ModuleLoadResult} defaultLoad
+ * @param {(url: string, context: ModuleLoadContext, result: ModuleLoadResult) => ModuleLoadResult} validateLoad
  * @returns {ModuleLoadResult}
  */
-function loadWithHooks(url, originalFormat, importAttributes, conditions, defaultLoad) {
+function loadWithHooks(url, originalFormat, importAttributes, conditions, defaultLoad, validateLoad) {
   debug('loadWithHooks', url, originalFormat);
   const context = new ModuleLoadContext(originalFormat, importAttributes, conditions);
   if (loadHooks.length === 0) {
@@ -403,4 +425,6 @@ module.exports = {
   registerHooks,
   resolveHooks,
   resolveWithHooks,
+  validateLoadStrict,
+  validateLoadSloppy,
 };

lib/internal/modules/esm/hooks.js

@@ -61,7 +61,7 @@ const {
   SHARED_MEMORY_BYTE_LENGTH,
   WORKER_TO_MAIN_THREAD_NOTIFICATION,
 } = require('internal/modules/esm/shared_constants');
-let debug = require('internal/util/debuglog').debuglog('esm', (fn) => {
+let debug = require('internal/util/debuglog').debuglog('async_loader_worker', (fn) => {
   debug = fn;
 });
 let importMetaInitializer;

lib/internal/modules/esm/load.js

@@ -141,15 +141,26 @@ function defaultLoadSync(url, context = kEmptyObject) {
 
   throwIfUnsupportedURLScheme(urlInstance, false);
 
+  let shouldBeReloadedByCJSLoader = false;
   if (urlInstance.protocol === 'node:') {
     source = null;
-  } else if (source == null) {
-    ({ responseURL, source } = getSourceSync(urlInstance, context));
-    context.source = source;
+    format ??= 'builtin';
+  } else if (format === 'addon') {
+    // Skip loading addon file content. It must be loaded with dlopen from file system.
+    source = null;
+  } else {
+    if (source == null) {
+      ({ responseURL, source } = getSourceSync(urlInstance, context));
+      context = { __proto__: context, source };
+    }
+
+    // Now that we have the source for the module, run `defaultGetFormat` to detect its format.
+    format ??= defaultGetFormat(urlInstance, context);
+
+    // For backward compatibility reasons, we need to let go through Module._load
+    // again.
+    shouldBeReloadedByCJSLoader = (format === 'commonjs');
   }
-
-  format ??= defaultGetFormat(urlInstance, context);
-
   validateAttributes(url, format, importAttributes);
 
   return {
@@ -157,6 +168,7 @@ function defaultLoadSync(url, context = kEmptyObject) {
     format,
     responseURL,
     source,
+    shouldBeReloadedByCJSLoader,
   };
 }
 

lib/internal/modules/esm/loader.js

@@ -55,8 +55,9 @@ const {
   resolveWithHooks,
   loadHooks,
   loadWithHooks,
+  validateLoadSloppy,
 } = require('internal/modules/customization_hooks');
-let defaultResolve, defaultLoad, defaultLoadSync, importMetaInitializer;
+let defaultResolve, defaultLoadSync, importMetaInitializer;
 
 const { tracingChannel } = require('diagnostics_channel');
 const onImport = tracingChannel('module.import');
@@ -146,6 +147,10 @@ let hooksProxy;
  * @typedef {ArrayBuffer|TypedArray|string} ModuleSource
  */
 
+/**
+ * @typedef {{ format: ModuleFormat, source: ModuleSource, translatorKey: string }} TranslateContext
+ */
+
 /**
  * This class covers the base machinery of module loading. To add custom
  * behavior you can pass a customizations object and this object will be
@@ -503,18 +508,19 @@ class ModuleLoader {
 
     const loadResult = this.#loadSync(url, { format, importAttributes });
 
+    const formatFromLoad = loadResult.format;
     // Use the synchronous commonjs translator which can deal with cycles.
-    const finalFormat =
-      loadResult.format === 'commonjs' ||
-      loadResult.format === 'commonjs-typescript' ? 'commonjs-sync' : loadResult.format;
+    const translatorKey = (formatFromLoad === 'commonjs' || formatFromLoad === 'commonjs-typescript') ?
+      'commonjs-sync' : formatFromLoad;
 
-    if (finalFormat === 'wasm') {
+    if (translatorKey === 'wasm') {
       assert.fail('WASM is currently unsupported by require(esm)');
     }
 
     const { source } = loadResult;
     const isMain = (parentURL === undefined);
-    const wrap = this.#translate(url, finalFormat, source, parentURL);
+    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) {
@@ -523,7 +529,7 @@ class ModuleLoader {
 
     const cjsModule = wrap[imported_cjs_symbol];
     if (cjsModule) {
-      assert(finalFormat === 'commonjs-sync');
+      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);
@@ -547,22 +553,22 @@ class ModuleLoader {
    * Translate a loaded module source into a ModuleWrap. This is run synchronously,
    * but the translator may return the ModuleWrap in a Promise.
    * @param {string} url URL of the module to be translated.
-   * @param {string} format Format of the module to be translated. This is used to find
-   *   matching translators.
-   * @param {ModuleSource} source Source of the module to be translated.
-   * @param {string|undefined} parentURL URL of the parent module. Undefined if it's the entry point.
+   * @param {TranslateContext} translateContext Context for the translator
+   * @param {string|undefined} parentURL URL of the module initiating the module loading for the first time.
+   *   Undefined if it's the entry point.
    * @returns {ModuleWrap}
    */
-  #translate(url, format, source, parentURL) {
+  #translate(url, translateContext, parentURL) {
+    const { translatorKey, format } = translateContext;
     this.validateLoadResult(url, format);
-    const translator = getTranslators().get(format);
+    const translator = getTranslators().get(translatorKey);
 
     if (!translator) {
-      throw new ERR_UNKNOWN_MODULE_FORMAT(format, url);
+      throw new ERR_UNKNOWN_MODULE_FORMAT(translatorKey, url);
     }
 
-    const result = FunctionPrototypeCall(translator, this, url, source, parentURL === undefined);
-    assert(result instanceof ModuleWrap);
+    const result = FunctionPrototypeCall(translator, this, url, translateContext, parentURL);
+    assert(result instanceof ModuleWrap, `The ${format} module returned is not a ModuleWrap`);
     return result;
   }
 
@@ -575,7 +581,8 @@ class ModuleLoader {
    * @returns {ModuleWrap}
    */
   loadAndTranslateForRequireInImportedCJS(url, loadContext, parentURL) {
-    const { format: formatFromLoad, source } = this.#loadSync(url, loadContext);
+    const loadResult = this.#loadSync(url, loadContext);
+    const formatFromLoad = loadResult.format;
 
     if (formatFromLoad === 'wasm') {  // require(wasm) is not supported.
       throw new ERR_UNKNOWN_MODULE_FORMAT(formatFromLoad, url);
@@ -587,15 +594,16 @@ class ModuleLoader {
       }
     }
 
-    let finalFormat = formatFromLoad;
+    let translatorKey = formatFromLoad;
     if (formatFromLoad === 'commonjs') {
-      finalFormat = 'require-commonjs';
+      translatorKey = 'require-commonjs';
     }
     if (formatFromLoad === 'commonjs-typescript') {
-      finalFormat = 'require-commonjs-typescript';
+      translatorKey = 'require-commonjs-typescript';
     }
 
-    const wrap = this.#translate(url, finalFormat, source, parentURL);
+    const translateContext = { ...loadResult, translatorKey, __proto__: null };
+    const wrap = this.#translate(url, translateContext, parentURL);
     assert(wrap instanceof ModuleWrap, `Translator used for require(${url}) should not be async`);
     return wrap;
   }
@@ -610,8 +618,9 @@ class ModuleLoader {
    */
   loadAndTranslate(url, loadContext, parentURL) {
     const maybePromise = this.load(url, loadContext);
-    const afterLoad = ({ format, source }) => {
-      return this.#translate(url, format, source, parentURL);
+    const afterLoad = (loadResult) => {
+      const translateContext = { ...loadResult, translatorKey: loadResult.format, __proto__: null };
+      return this.#translate(url, translateContext, parentURL);
     };
     if (isPromise(maybePromise)) {
       return maybePromise.then(afterLoad);
@@ -837,8 +846,8 @@ class ModuleLoader {
       return this.#customizations.load(url, context);
     }
 
-    defaultLoad ??= require('internal/modules/esm/load').defaultLoad;
-    return defaultLoad(url, context);
+    defaultLoadSync ??= require('internal/modules/esm/load').defaultLoadSync;
+    return defaultLoadSync(url, context);
   }
 
   /**
@@ -873,7 +882,7 @@ class ModuleLoader {
       // TODO(joyeecheung): construct the ModuleLoadContext in the loaders directly instead
       // of converting them from plain objects in the hooks.
       return loadWithHooks(url, context.format, context.importAttributes, this.#defaultConditions,
-                           this.#loadAndMaybeBlockOnLoaderThread.bind(this));
+                           this.#loadAndMaybeBlockOnLoaderThread.bind(this), validateLoadSloppy);
     }
     return this.#loadAndMaybeBlockOnLoaderThread(url, context);
   }

lib/internal/modules/esm/translators.js

@@ -100,10 +100,12 @@ function errPath(url) {
 }
 
 // Strategy for loading a standard JavaScript module.
-translators.set('module', function moduleStrategy(url, source, isMain) {
+translators.set('module', function moduleStrategy(url, translateContext, parentURL) {
+  let { source } = translateContext;
+  const isMain = (parentURL === undefined);
   assertBufferSource(source, true, 'load');
   source = stringify(source);
-  debug(`Translating StandardModule ${url}`);
+  debug(`Translating StandardModule ${url}`, translateContext);
   const { compileSourceTextModule } = require('internal/modules/esm/utils');
   const context = isMain ? { isMain } : undefined;
   const module = compileSourceTextModule(url, source, this, context);
@@ -199,20 +201,23 @@ const cjsCache = new SafeMap();
 /**
  * Creates a ModuleWrap object for a CommonJS module.
  * @param {string} url - The URL of the module.
- * @param {string} source - The source code of the module.
- * @param {boolean} isMain - Whether the module is the main module.
- * @param {string} format - Format of the module.
+ * @param {{ format: ModuleFormat, source: ModuleSource }} translateContext Context for the translator
+ * @param {string|undefined} parentURL URL of the module initiating the module loading for the first time.
+ *   Undefined if it's the entry point.
  * @param {typeof loadCJSModule} [loadCJS] - The function to load the CommonJS module.
  * @returns {ModuleWrap} The ModuleWrap object for the CommonJS module.
  */
-function createCJSModuleWrap(url, source, isMain, format, loadCJS = loadCJSModule) {
-  debug(`Translating CJSModule ${url}`);
+function createCJSModuleWrap(url, translateContext, parentURL, loadCJS = loadCJSModule) {
+  debug(`Translating CJSModule ${url}`, translateContext);
 
+  const { format: sourceFormat } = translateContext;
+  let { source } = translateContext;
+  const isMain = (parentURL === undefined);
   const filename = urlToFilename(url);
   // In case the source was not provided by the `load` step, we need fetch it now.
   source = stringify(source ?? getSource(new URL(url)).source);
 
-  const { exportNames, module } = cjsPreparseModuleExports(filename, source, format);
+  const { exportNames, module } = cjsPreparseModuleExports(filename, source, sourceFormat);
   cjsCache.set(url, module);
 
   const wrapperNames = [...exportNames];
@@ -263,11 +268,12 @@ function createCJSModuleWrap(url, source, isMain, format, loadCJS = loadCJSModul
 /**
  * Creates a ModuleWrap object for a CommonJS module without source texts.
  * @param {string} url - The URL of the module.
- * @param {boolean} isMain - Whether the module is the main module.
+ * @param {string|undefined} parentURL - URL of the parent module, if any.
  * @returns {ModuleWrap} The ModuleWrap object for the CommonJS module.
  */
-function createCJSNoSourceModuleWrap(url, isMain) {
+function createCJSNoSourceModuleWrap(url, parentURL) {
   debug(`Translating CJSModule without source ${url}`);
+  const isMain = (parentURL === undefined);
 
   const filename = urlToFilename(url);
 
@@ -301,54 +307,60 @@ function createCJSNoSourceModuleWrap(url, isMain) {
   }, module);
 }
 
-translators.set('commonjs-sync', function requireCommonJS(url, source, isMain) {
+translators.set('commonjs-sync', function requireCommonJS(url, translateContext, parentURL) {
   initCJSParseSync();
 
-  return createCJSModuleWrap(url, source, isMain, 'commonjs', (module, source, url, filename, isMain) => {
-    assert(module === CJSModule._cache[filename]);
-    wrapModuleLoad(filename, null, isMain);
-  });
+  return createCJSModuleWrap(url, translateContext, parentURL, loadCJSModuleWithModuleLoad);
 });
 
 // Handle CommonJS modules referenced by `require` calls.
 // This translator function must be sync, as `require` is sync.
-translators.set('require-commonjs', (url, source, isMain) => {
+translators.set('require-commonjs', (url, translateContext, parentURL) => {
   initCJSParseSync();
   assert(cjsParse);
 
-  return createCJSModuleWrap(url, source, isMain, 'commonjs');
+  return createCJSModuleWrap(url, translateContext, parentURL);
 });
 
 // Handle CommonJS modules referenced by `require` calls.
 // This translator function must be sync, as `require` is sync.
-translators.set('require-commonjs-typescript', (url, source, isMain) => {
+translators.set('require-commonjs-typescript', (url, translateContext, parentURL) => {
   assert(cjsParse);
-  const code = stripTypeScriptModuleTypes(stringify(source), url);
-  return createCJSModuleWrap(url, code, isMain, 'commonjs-typescript');
+  translateContext.source = stripTypeScriptModuleTypes(stringify(translateContext.source), url);
+  return createCJSModuleWrap(url, translateContext, parentURL);
 });
 
+// This goes through Module._load to accommodate monkey-patchers.
+function loadCJSModuleWithModuleLoad(module, source, url, filename, isMain) {
+  assert(module === CJSModule._cache[filename]);
+  wrapModuleLoad(filename, undefined, isMain);
+}
+
 // Handle CommonJS modules referenced by `import` statements or expressions,
 // or as the initial entry point when the ESM loader handles a CommonJS entry.
-translators.set('commonjs', function commonjsStrategy(url, source, isMain) {
+translators.set('commonjs', function commonjsStrategy(url, translateContext, parentURL) {
   if (!cjsParse) {
     initCJSParseSync();
   }
 
   // For backward-compatibility, it's possible to return a nullish value for
-  // CJS source associated with a file: URL. In this case, the source is
-  // obtained by calling the monkey-patchable CJS loader.
-  const cjsLoader = source == null ? (module, source, url, filename, isMain) => {
-    assert(module === CJSModule._cache[filename]);
-    wrapModuleLoad(filename, undefined, isMain);
-  } : loadCJSModule;
+  // CJS source associated with a `file:` URL - that usually means the source is not
+  // customized (is loaded by default load) or the hook author wants it to be reloaded
+  // through CJS routine. In this case, the source is obtained by calling the
+  // monkey-patchable CJS loader.
+  // TODO(joyeecheung): just use wrapModuleLoad and let the CJS loader
+  // invoke the off-thread hooks. Use a special parent to avoid invoking in-thread
+  // hooks twice.
+  const shouldReloadByCJSLoader = (translateContext.shouldBeReloadedByCJSLoader || translateContext.source == null);
+  const cjsLoader = shouldReloadByCJSLoader ? loadCJSModuleWithModuleLoad : loadCJSModule;
 
   try {
     // We still need to read the FS to detect the exports.
-    source ??= readFileSync(new URL(url), 'utf8');
+    translateContext.source ??= readFileSync(new URL(url), 'utf8');
   } catch {
     // Continue regardless of error.
   }
-  return createCJSModuleWrap(url, source, isMain, 'commonjs', cjsLoader);
+  return createCJSModuleWrap(url, translateContext, parentURL, cjsLoader);
 });
 
 /**
@@ -373,22 +385,6 @@ function cjsEmplaceModuleCacheEntry(filename, parent) {
   return cjsMod;
 }
 
-/**
- * Emplace a CJS module cache entry for the given URL.
- * @param {string} url The module URL
- * @param {CJSModule} parent The parent CJS module
- * @returns {CJSModule|undefined} the cached CJS module entry, undefined if url cannot be used to identify a CJS entry.
- */
-exports.cjsEmplaceModuleCacheEntryForURL = function cjsEmplaceModuleCacheEntryForURL(url, parent) {
-  const filename = urlToFilename(url);
-  if (!filename) {
-    return;
-  }
-  const cjsModule = cjsEmplaceModuleCacheEntry(filename, parent);
-  cjsCache.set(url, cjsModule);
-  return cjsModule;
-};
-
 /**
  * Pre-parses a CommonJS module's exports and re-exports.
  * @param {string} filename - The filename of the module.
@@ -454,8 +450,8 @@ function cjsPreparseModuleExports(filename, source, format) {
 
 // Strategy for loading a node builtin CommonJS module that isn't
 // through normal resolution
-translators.set('builtin', function builtinStrategy(url) {
-  debug(`Translating BuiltinModule ${url}`);
+translators.set('builtin', function builtinStrategy(url, translateContext) {
+  debug(`Translating BuiltinModule ${url}`, translateContext);
   // Slice 'node:' scheme
   const id = StringPrototypeSlice(url, 5);
   const module = loadBuiltinModule(id, url);
@@ -468,7 +464,8 @@ translators.set('builtin', function builtinStrategy(url) {
 });
 
 // Strategy for loading a JSON file
-translators.set('json', function jsonStrategy(url, source) {
+translators.set('json', function jsonStrategy(url, translateContext) {
+  let { source } = translateContext;
   assertBufferSource(source, true, 'load');
   debug(`Loading JSONModule ${url}`);
   const pathname = StringPrototypeStartsWith(url, 'file:') ?
@@ -536,10 +533,11 @@ translators.set('json', function jsonStrategy(url, source) {
  * >} [[Instance]] slot proxy for WebAssembly Module Record
  */
 const wasmInstances = new SafeWeakMap();
-translators.set('wasm', function(url, source) {
+translators.set('wasm', function(url, translateContext) {
+  const { source } = translateContext;
   assertBufferSource(source, false, 'load');
 
-  debug(`Translating WASMModule ${url}`);
+  debug(`Translating WASMModule ${url}`, translateContext);
 
   let compiled;
   try {
@@ -626,9 +624,10 @@ translators.set('wasm', function(url, source) {
 });
 
 // Strategy for loading a addon
-translators.set('addon', function translateAddon(url, source, isMain) {
+translators.set('addon', function translateAddon(url, translateContext, parentURL) {
   emitExperimentalWarning('Importing addons');
 
+  const { source } = translateContext;
   // The addon must be loaded from file system with dlopen. Assert
   // the source is null.
   if (source !== null) {
@@ -639,23 +638,25 @@ translators.set('addon', function translateAddon(url, source, isMain) {
       source);
   }
 
-  debug(`Translating addon ${url}`);
+  debug(`Translating addon ${url}`, translateContext);
 
-  return createCJSNoSourceModuleWrap(url, isMain);
+  return createCJSNoSourceModuleWrap(url, parentURL);
 });
 
 // Strategy for loading a commonjs TypeScript module
-translators.set('commonjs-typescript', function(url, source, isMain) {
+translators.set('commonjs-typescript', function(url, translateContext, parentURL) {
+  const { source } = translateContext;
   assertBufferSource(source, true, 'load');
-  const code = stripTypeScriptModuleTypes(stringify(source), url);
-  debug(`Translating TypeScript ${url}`);
-  return FunctionPrototypeCall(translators.get('commonjs'), this, url, code, isMain);
+  debug(`Translating TypeScript ${url}`, translateContext);
+  translateContext.source = stripTypeScriptModuleTypes(stringify(source), url);
+  return FunctionPrototypeCall(translators.get('commonjs'), this, url, translateContext, parentURL);
 });
 
 // Strategy for loading an esm TypeScript module
-translators.set('module-typescript', function(url, source, isMain) {
+translators.set('module-typescript', function(url, translateContext, parentURL) {
+  const { source } = translateContext;
   assertBufferSource(source, true, 'load');
-  const code = stripTypeScriptModuleTypes(stringify(source), url);
-  debug(`Translating TypeScript ${url}`);
-  return FunctionPrototypeCall(translators.get('module'), this, url, code, isMain);
+  debug(`Translating TypeScript ${url}`, translateContext);
+  translateContext.source = stripTypeScriptModuleTypes(stringify(source), url);
+  return FunctionPrototypeCall(translators.get('module'), this, url, translateContext, parentURL);
 });

test/fixtures/module-hooks/sync-and-async/app.js

@@ -0,0 +1,2 @@
+console.log('Hello world');
+

test/fixtures/module-hooks/sync-and-async/async-customize-loader.js

@@ -0,0 +1,10 @@
+export async function load(url, context, nextLoad) {
+  if (url.endsWith('app.js')) {
+    return {
+      shortCircuit: true,
+      format: 'module',
+      source: 'console.log("customized by async hook");',
+    };
+  }
+  return nextLoad(url, context);
+}

test/fixtures/module-hooks/sync-and-async/async-customize.js

@@ -0,0 +1,3 @@
+import { register } from 'node:module';
+
+register(new URL('async-customize-loader.js', import.meta.url));

test/fixtures/module-hooks/sync-and-async/async-forward-loader.js

@@ -0,0 +1,3 @@
+export async function load(url, context, nextLoad) {
+  return nextLoad(url, context);
+}

test/fixtures/module-hooks/sync-and-async/async-forward.js

@@ -0,0 +1,3 @@
+import { register } from 'node:module';
+
+register(new URL('async-forward-loader.js', import.meta.url));

test/fixtures/module-hooks/sync-and-async/sync-customize.js

@@ -0,0 +1,14 @@
+import { registerHooks } from 'node:module';
+
+registerHooks({
+  load(url, context, nextLoad) {
+    if (url.endsWith('app.js')) {
+      return {
+        shortCircuit: true,
+        format: 'module',
+        source: 'console.log("customized by sync hook")',
+      };
+    }
+    return nextLoad(url, context);
+  },
+});

test/fixtures/module-hooks/sync-and-async/sync-forward.js

@@ -0,0 +1,7 @@
+import { registerHooks } from 'node:module';
+
+registerHooks({
+  load(url, context, nextLoad) {
+    return nextLoad(url, context);
+  },
+});

test/module-hooks/test-module-hooks-load-async-and-sync.js

@@ -0,0 +1,32 @@
+'use strict';
+// This tests that sync and async hooks can be mixed.
+
+require('../common');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const fixtures = require('../common/fixtures');
+
+const app = fixtures.path('module-hooks', 'sync-and-async', 'app.js');
+
+const testCases = [
+  // When mixing sync and async hooks, the sync ones always run first.
+  { preload: ['sync-customize', 'async-customize'], stdout: 'customized by sync hook' },
+  { preload: ['async-customize', 'sync-customize'], stdout: 'customized by sync hook' },
+  // It should still work when neither hook does any customization.
+  { preload: ['sync-forward', 'async-forward'], stdout: 'Hello world' },
+  { preload: ['async-forward', 'sync-forward'], stdout: 'Hello world' },
+  // It should work when only one hook is customizing.
+  { preload: ['sync-customize', 'async-forward'], stdout: 'customized by sync hook' },
+  { preload: ['async-customize', 'sync-forward'], stdout: 'customized by async hook' },
+];
+
+
+for (const { preload, stdout } of testCases) {
+  const importArgs = [];
+  for (const p of preload) {
+    importArgs.push('--import', fixtures.fileURL(`module-hooks/sync-and-async/${p}.js`));
+  }
+  spawnSyncAndAssert(process.execPath, [...importArgs, app], {
+    stdout,
+    trim: true,
+  });
+}