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

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,29 +262,56 @@ 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,
   // and any source content for builtins provided by the user hooks are ignored.
   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',
       'source',
       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,22 +141,34 @@ 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 };
+    }
 
-  format ??= defaultGetFormat(urlInstance, context);
+    // 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');
+  }
   validateAttributes(url, format, importAttributes);
 
   return {
     __proto__: null,
     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);
   }