[v22.x] backport module loader & loader hook fixes

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

@@ -145,22 +145,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

@@ -53,8 +53,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');
@@ -89,13 +90,18 @@ function newLoadCache() {
   return new LoadCache();
 }
 
+let _translators;
+function lazyLoadTranslators() {
+  _translators ??= require('internal/modules/esm/translators');
+  return _translators;
+}
+
 /**
  * Lazy-load translators to avoid potentially unnecessary work at startup (ex if ESM is not used).
  * @returns {import('./translators.js').Translators}
  */
 function getTranslators() {
-  const { translators } = require('internal/modules/esm/translators');
-  return translators;
+  return lazyLoadTranslators().translators;
 }
 
 /**
@@ -139,6 +145,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
@@ -487,16 +497,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' ? '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, isMain);
+    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) {
@@ -505,7 +518,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);
@@ -529,22 +542,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 {boolean} isMain Whether the module to be translated is 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, isMain) {
+  #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, isMain);
-    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;
   }
 
@@ -553,11 +566,12 @@ class ModuleLoader {
    * 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 {boolean} isMain Whether the module to be translated is the entry point.
+   * @param {string|undefined} parentURL URL of the parent module. Undefined if it's the entry point.
    * @returns {ModuleWrap}
    */
-  loadAndTranslateForRequireInImportedCJS(url, loadContext, isMain) {
-    const { format: formatFromLoad, source } = this.#loadSync(url, loadContext);
+  loadAndTranslateForRequireInImportedCJS(url, loadContext, parentURL) {
+    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);
@@ -569,15 +583,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, isMain);
+    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;
   }
@@ -587,13 +602,14 @@ class ModuleLoader {
    * This may be run asynchronously if there are asynchronous module loader hooks registered.
    * @param {string} url URL of the module to be translated.
    * @param {object} loadContext See {@link load}
-   * @param {boolean} isMain Whether the module to be translated is the entry point.
+   * @param {string|undefined} parentURL URL of the parent module. Undefined if it's the entry point.
    * @returns {Promise<ModuleWrap>|ModuleWrap}
    */
-  loadAndTranslate(url, loadContext, isMain) {
+  loadAndTranslate(url, loadContext, parentURL) {
     const maybePromise = this.load(url, loadContext);
-    const afterLoad = ({ format, source }) => {
-      return this.#translate(url, format, source, isMain);
+    const afterLoad = (loadResult) => {
+      const translateContext = { ...loadResult, translatorKey: loadResult.format, __proto__: null };
+      return this.#translate(url, translateContext, parentURL);
     };
     if (isPromise(maybePromise)) {
       return maybePromise.then(afterLoad);
@@ -619,9 +635,9 @@ class ModuleLoader {
     const isMain = parentURL === undefined;
     let moduleOrModulePromise;
     if (isForRequireInImportedCJS) {
-      moduleOrModulePromise = this.loadAndTranslateForRequireInImportedCJS(url, context, isMain);
+      moduleOrModulePromise = this.loadAndTranslateForRequireInImportedCJS(url, context, parentURL);
     } else {
-      moduleOrModulePromise = this.loadAndTranslate(url, context, isMain);
+      moduleOrModulePromise = this.loadAndTranslate(url, context, parentURL);
     }
 
     const inspectBrk = (
@@ -811,8 +827,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);
   }
 
   /**
@@ -847,7 +863,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);
   }