From c471c5e26e0aeec165113a08d5b0a0f8be162a23 Mon Sep 17 00:00:00 2001
From: Guy Bedford <guybedford@gmail.com>
Date: Sun, 29 May 2022 15:58:48 -0700
Subject: [PATCH 1/2] module: add preImport loader hook

---
 doc/api/esm.md                          |  54 +++++-
 lib/internal/modules/cjs/loader.js      |  10 +-
 lib/internal/modules/esm/loader.js      | 248 ++++++++++--------------
 lib/internal/modules/esm/translators.js |   2 +-
 lib/internal/process/esm_loader.js      |  18 +-
 lib/internal/process/execution.js       |   2 +-
 lib/repl.js                             |   4 +-
 7 files changed, 176 insertions(+), 162 deletions(-)

diff --git a/doc/api/esm.md b/doc/api/esm.md
index c5ef7eb863f6b7..d62ca92845ea17 100644
--- a/doc/api/esm.md
+++ b/doc/api/esm.md
@@ -734,6 +734,54 @@ A hook that returns without calling `next<hookName>()` _and_ without returning
 `shortCircuit: true` also triggers an exception. These errors are to help
 prevent unintentional breaks in the chain.
 
+#### `preImport(specifier, context)`
+
+<!-- YAML
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/43245
+    description: Add support for preImport hook.
+-->
+
+> The loaders API is being redesigned. This hook may disappear or its
+> signature may change. Do not rely on the API described below.
+
+* `specifier` {string}
+* `context` {Object}
+  * `conditions` {string\[]} Resolution conditions of the current environment,
+    as defined for the `package.json` imports and exports fields
+  * `dynamic` {boolean} Whether this import is a dynamic `import()` (if *false*,
+    that means it is a host top-level import).
+  * `importAssertions` {Object}
+  * `parentURL` {string|undefined} The module importing this one, or undefined
+    if this is the Node.js entry point
+
+The `preImport` hook allows for tracking and asynchronous setup work for every
+top-level import operation. It has no return value, although it can return
+a promise to to delay the module pipeline operations. No further hooks will
+be called on that module graph until this hook resolves successfully if present.
+
+The `preImport` hook is called for each top-level import operation by the module
+loader, both for the host-called imports (ie for the main entry) and for dynamic
+`import()` imports. These are distinguished by the `dynamic` context.
+
+All `preImport` hooks for all loaders are run asynchronously in parallel, and
+block any further load operations (ie resolve and load) for the module graph
+being imported until they all complete successfully.
+
+Multiple import calls to the same import specifier will re-call the hook
+multiple times. The first error thrown by the `preImport` hooks will be directly
+returned to the specific import operation as the load failure.
+
+```js
+export async function preImport(specifier, context) {
+  if (context.topLevel)
+    console.log(`Top-level load of ${specifier}`);
+  else
+    console.log(`Dynamic import of ${specifier} in ${context.parentURL}`);
+}
+```
+
 #### `resolve(specifier, context, nextResolve)`
 
 <!-- YAML
@@ -758,7 +806,8 @@ changes:
 
 * `specifier` {string}
 * `context` {Object}
-  * `conditions` {string\[]} Export conditions of the relevant `package.json`
+  * `conditions` {string\[]} Resolution conditions of the current environment,
+    as defined for the `package.json` imports and exports fields
   * `importAssertions` {Object}
   * `parentURL` {string|undefined} The module importing this one, or undefined
     if this is the Node.js entry point
@@ -851,7 +900,8 @@ changes:
 
 * `url` {string} The URL returned by the `resolve` chain
 * `context` {Object}
-  * `conditions` {string\[]} Export conditions of the relevant `package.json`
+  * `conditions` {string\[]} Resolution conditions of the current environment,
+    as defined for the `package.json` imports and exports fields
   * `format` {string|null|undefined} The format optionally supplied by the
     `resolve` hook chain
   * `importAssertions` {Object}
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
index 11e029b258aa9a..34de00d50be942 100644
--- a/lib/internal/modules/cjs/loader.js
+++ b/lib/internal/modules/cjs/loader.js
@@ -1036,8 +1036,9 @@ function wrapSafe(filename, content, cjsModuleInstance) {
       displayErrors: true,
       importModuleDynamically: async (specifier, _, importAssertions) => {
         const loader = asyncESM.esmLoader;
-        return loader.import(specifier, normalizeReferrerURL(filename),
-                             importAssertions);
+        return loader.import(specifier,
+                             normalizeReferrerURL(filename),
+                             importAssertions, true);
       },
     });
   }
@@ -1052,8 +1053,9 @@ function wrapSafe(filename, content, cjsModuleInstance) {
       filename,
       importModuleDynamically(specifier, _, importAssertions) {
         const loader = asyncESM.esmLoader;
-        return loader.import(specifier, normalizeReferrerURL(filename),
-                             importAssertions);
+        return loader.import(specifier,
+                             normalizeReferrerURL(filename),
+                             importAssertions, true);
       },
     });
   } catch (err) {
diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
index ef9b32f4deb8ed..8cbf79650ed72b 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/lib/internal/modules/esm/loader.js
@@ -4,15 +4,15 @@
 require('internal/modules/cjs/loader');
 
 const {
-  Array,
-  ArrayIsArray,
   ArrayPrototypeJoin,
+  ArrayPrototypeMap,
   ArrayPrototypePush,
   FunctionPrototypeBind,
   FunctionPrototypeCall,
   ObjectAssign,
   ObjectCreate,
   ObjectDefineProperty,
+  ObjectFreeze,
   ObjectSetPrototypeOf,
   PromiseAll,
   PromiseResolve,
@@ -194,13 +194,20 @@ function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
  * the main module and everything in its dependency graph.
  */
 class ESMLoader {
+  /**
+   * Whether this loader has userland hooks.
+   * @private
+   * @property {boolean} hasLoaderHooks
+   */
+  #hasLoaderHooks = false;
+
   /**
    * Prior to ESM loading. These are called once before any modules are started.
    * @private
-   * @property {KeyedHook[]} globalPreloaders Last-in-first-out
+   * @property {KeyedHook[]} globalPreloadHooks Last-in-first-out
    *  list of preload hooks.
    */
-  #globalPreloaders = [];
+  #globalPreloadHooks = [];
 
   /**
    * Phase 2 of 2 in ESM loading.
@@ -208,20 +215,22 @@ class ESMLoader {
    * @property {KeyedHook[]} loaders Last-in-first-out
    *  collection of loader hooks.
    */
-  #loaders = [
+  #loadHooks = [
     {
       fn: defaultLoad,
       url: 'node:internal/modules/esm/load',
     },
   ];
 
+  #preImportHooks = [];
+
   /**
    * Phase 1 of 2 in ESM loading.
    * @private
    * @property {KeyedHook[]} resolvers Last-in-first-out
    *  collection of resolver hooks.
    */
-  #resolvers = [
+  #resolveHooks = [
     {
       fn: defaultResolve,
       url: 'node:internal/modules/esm/resolve',
@@ -269,71 +278,6 @@ class ESMLoader {
     }
   }
 
-  /**
-   *
-   * @param {ModuleExports} exports
-   * @returns {ExportedHooks}
-   */
-  static pluckHooks({
-    globalPreload,
-    resolve,
-    load,
-    // obsolete hooks:
-    dynamicInstantiate,
-    getFormat,
-    getGlobalPreloadCode,
-    getSource,
-    transformSource,
-  }) {
-    const obsoleteHooks = [];
-    const acceptedHooks = ObjectCreate(null);
-
-    if (getGlobalPreloadCode) {
-      globalPreload ??= getGlobalPreloadCode;
-
-      process.emitWarning(
-        'Loader hook "getGlobalPreloadCode" has been renamed to "globalPreload"'
-      );
-    }
-    if (dynamicInstantiate) ArrayPrototypePush(
-      obsoleteHooks,
-      'dynamicInstantiate'
-    );
-    if (getFormat) ArrayPrototypePush(
-      obsoleteHooks,
-      'getFormat',
-    );
-    if (getSource) ArrayPrototypePush(
-      obsoleteHooks,
-      'getSource',
-    );
-    if (transformSource) ArrayPrototypePush(
-      obsoleteHooks,
-      'transformSource',
-    );
-
-    if (obsoleteHooks.length) process.emitWarning(
-      `Obsolete loader hook(s) supplied and will be ignored: ${
-        ArrayPrototypeJoin(obsoleteHooks, ', ')
-      }`,
-      'DeprecationWarning',
-    );
-
-    // Use .bind() to avoid giving access to the Loader instance when called.
-    if (globalPreload) {
-      acceptedHooks.globalPreloader =
-        FunctionPrototypeBind(globalPreload, null);
-    }
-    if (resolve) {
-      acceptedHooks.resolver = FunctionPrototypeBind(resolve, null);
-    }
-    if (load) {
-      acceptedHooks.loader = FunctionPrototypeBind(load, null);
-    }
-
-    return acceptedHooks;
-  }
-
   /**
    * Collect custom/user-defined hook(s). After all hooks have been collected,
    * calls global preload hook(s).
@@ -349,43 +293,83 @@ class ESMLoader {
         exports,
         url,
       } = customLoaders[i];
-      const {
-        globalPreloader,
-        resolver,
-        loader,
-      } = ESMLoader.pluckHooks(exports);
 
-      if (globalPreloader) {
+      const obsoleteHooks = [];
+
+      if (exports.getGlobalPreloadCode) {
+        process.emitWarning(
+          'exports hook "getGlobalPreloadCode" has been renamed to "globalPreload"'
+        );
+      }
+      if (exports.dynamicInstantiate) ArrayPrototypePush(
+        obsoleteHooks,
+        'dynamicInstantiate'
+      );
+      if (exports.getFormat) ArrayPrototypePush(
+        obsoleteHooks,
+        'getFormat',
+      );
+      if (exports.getSource) ArrayPrototypePush(
+        obsoleteHooks,
+        'getSource',
+      );
+      if (exports.transformSource) ArrayPrototypePush(
+        obsoleteHooks,
+        'transformSource',
+      );
+
+      if (obsoleteHooks.length) process.emitWarning(
+        `Obsolete loader hook(s) supplied and will be ignored: ${
+          ArrayPrototypeJoin(obsoleteHooks, ', ')
+        }`,
+        'DeprecationWarning',
+      );
+
+      const globalPreloadHook = exports.getGlobalPreloadCode ?? exports.globalPreload;
+      const preImportHook = exports.preImport;
+      const resolveHook = exports.resolve;
+      const loadHook = exports.load;
+
+      // [1] ensure hook function is not bound to ESMLoader instance
+      if (globalPreloadHook) {
+        this.#hasLoaderHooks = true;
         ArrayPrototypePush(
-          this.#globalPreloaders,
+          this.#globalPreloadHooks,
           {
-            fn: FunctionPrototypeBind(globalPreloader), // [1]
+            fn: FunctionPrototypeBind(globalPreloadHook), // [1]
             url,
           },
         );
       }
-      if (resolver) {
+      if (preImportHook) {
+        this.#hasLoaderHooks = true;
+        ArrayPrototypePush(
+          this.#preImportHooks,
+          FunctionPrototypeBind(preImportHook), // [1]
+        );
+      }
+      if (resolveHook) {
+        this.#hasLoaderHooks = true;
         ArrayPrototypePush(
-          this.#resolvers,
+          this.#resolveHooks,
           {
-            fn: FunctionPrototypeBind(resolver), // [1]
+            fn: FunctionPrototypeBind(resolveHook), // [1]
             url,
           },
         );
       }
-      if (loader) {
+      if (loadHook) {
+        this.#hasLoaderHooks = true;
         ArrayPrototypePush(
-          this.#loaders,
+          this.#loadHooks,
           {
-            fn: FunctionPrototypeBind(loader), // [1]
+            fn: FunctionPrototypeBind(loadHook), // [1]
             url,
           },
         );
       }
     }
 
-    // [1] ensure hook function is not bound to ESMLoader instance
-
     this.preload();
   }
 
@@ -398,7 +382,7 @@ class ESMLoader {
       const module = new ModuleWrap(url, undefined, source, 0, 0);
       callbackMap.set(module, {
         importModuleDynamically: (specifier, { url }, importAssertions) => {
-          return this.import(specifier, url, importAssertions);
+          return this.import(specifier, url, importAssertions, true);
         }
       });
 
@@ -428,22 +412,10 @@ class ESMLoader {
    * @returns {Promise<ModuleJob>} The (possibly pending) module job
    */
   async getModuleJob(specifier, parentURL, importAssertions) {
-    let importAssertionsForResolve;
-
-    // By default, `this.#loaders` contains just the Node default load hook
-    if (this.#loaders.length !== 1) {
-      // We can skip cloning if there are no user-provided loaders because
-      // the Node.js default resolve hook does not use import assertions.
-      importAssertionsForResolve = ObjectAssign(
-        ObjectCreate(null),
-        importAssertions,
-      );
-    }
-
     const { format, url } = this.resolve(
       specifier,
       parentURL,
-      importAssertionsForResolve,
+      importAssertions,
     );
 
     let job = this.moduleMap.get(url, importAssertions.type);
@@ -517,48 +489,29 @@ class ESMLoader {
    * This method must NOT be renamed: it functions as a dynamic import on a
    * loader module.
    *
-   * @param {string | string[]} specifiers Path(s) to the module.
-   * @param {string} parentURL Path of the parent importing the module.
+   * @param {string} specifier Imported specifier
+   * @param {string} parentURL URL of the parent importing the module.
    * @param {Record<string, string>} importAssertions Validations for the
    *                                                  module import.
-   * @returns {Promise<ExportedHooks | KeyedExports[]>}
-   *  A collection of module export(s) or a list of collections of module
-   *  export(s).
+   * @param {boolean} dynamic Whether the import is a dynamic `import()`.
+   * @returns {Promise<ModuleNamespace>}
    */
-  async import(specifiers, parentURL, importAssertions) {
-    // For loaders, `import` is passed multiple things to process, it returns a
-    // list pairing the url and exports collected. This is especially useful for
-    // error messaging, to identity from where an export came. But, in most
-    // cases, only a single url is being "imported" (ex `import()`), so there is
-    // only 1 possible url from which the exports were collected and it is
-    // already known to the caller. Nesting that in a list would only ever
-    // create redundant work for the caller, so it is later popped off the
-    // internal list.
-    const wasArr = ArrayIsArray(specifiers);
-    if (!wasArr) { specifiers = [specifiers]; }
-
-    const count = specifiers.length;
-    const jobs = new Array(count);
-
-    for (let i = 0; i < count; i++) {
-      jobs[i] = this.getModuleJob(specifiers[i], parentURL, importAssertions)
-        .then((job) => job.run())
-        .then(({ module }) => module.getNamespace());
-    }
-
-    const namespaces = await PromiseAll(new SafeArrayIterator(jobs));
-
-    if (!wasArr) { return namespaces[0]; } // We can skip the pairing below
-
-    for (let i = 0; i < count; i++) {
-      const namespace = ObjectCreate(null);
-      namespace.url = specifiers[i];
-      namespace.exports = namespaces[i];
-
-      namespaces[i] = namespace;
-    }
-
-    return namespaces;
+  async import(specifier, parentURL,
+               importAssertions = ObjectCreate(null), dynamic = false) {
+    const context = ObjectFreeze({
+      conditions: DEFAULT_CONDITIONS,
+      dynamic,
+      importAssertions: this.#hasLoaderHooks ?
+          ObjectAssign(ObjectCreate(null), importAssertions) : importAssertions,
+      parentURL
+    });
+    await PromiseAll(new SafeArrayIterator(ArrayPrototypeMap(
+      this.#preImportHooks,
+      (preImportHook) => preImportHook(specifier, context)
+    )));
+    const job = await this.getModuleJob(specifier, parentURL, importAssertions);
+    const { module } = await job.run();
+    return module.getNamespace();
   }
 
   /**
@@ -573,7 +526,7 @@ class ESMLoader {
    * @returns {{ format: ModuleFormat, source: ModuleSource }}
    */
   async load(url, context = {}) {
-    const chain = this.#loaders;
+    const chain = this.#loadHooks;
     const meta = {
       chainFinished: null,
       hookErrIdentifier: '',
@@ -702,7 +655,7 @@ class ESMLoader {
   }
 
   preload() {
-    for (let i = this.#globalPreloaders.length - 1; i >= 0; i--) {
+    for (let i = this.#globalPreloadHooks.length - 1; i >= 0; i--) {
       const channel = new MessageChannel();
       const {
         port1: insidePreload,
@@ -715,7 +668,7 @@ class ESMLoader {
       const {
         fn: preloader,
         url: specifier,
-      } = this.#globalPreloaders[i];
+      } = this.#globalPreloadHooks[i];
 
       const preload = preloader({
         port: insideLoader,
@@ -822,7 +775,7 @@ class ESMLoader {
         parentURL,
       );
     }
-    const chain = this.#resolvers;
+    const chain = this.#resolveHooks;
     const meta = {
       chainFinished: null,
       hookErrIdentifier: '',
@@ -831,11 +784,12 @@ class ESMLoader {
       isChainAsync: false,
       shortCircuited: false,
     };
-    const context = {
+    const context = ObjectFreeze({
       conditions: DEFAULT_CONDITIONS,
-      importAssertions,
+      importAssertions: this.#hasLoaderHooks ?
+          ObjectAssign(ObjectCreate(null), importAssertions) : importAssertions,
       parentURL,
-    };
+    });
 
     const validateArgs = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {
       validateString(
diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
index bcd1775bac898e..800f707452b623 100644
--- a/lib/internal/modules/esm/translators.js
+++ b/lib/internal/modules/esm/translators.js
@@ -103,7 +103,7 @@ function errPath(url) {
 }
 
 async function importModuleDynamically(specifier, { url }, assertions) {
-  return asyncESM.esmLoader.import(specifier, url, assertions);
+  return asyncESM.esmLoader.import(specifier, url, assertions, true);
 }
 
 // Strategy for loading a standard JavaScript module.
diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js
index 4634ff5a9f5101..b87d2b41281950 100644
--- a/lib/internal/process/esm_loader.js
+++ b/lib/internal/process/esm_loader.js
@@ -1,7 +1,10 @@
 'use strict';
 
 const {
+  ArrayPrototypeMap,
   ObjectCreate,
+  PromiseAll,
+  SafeArrayIterator,
 } = primordials;
 
 const {
@@ -66,11 +69,16 @@ async function initializeLoader() {
   const internalEsmLoader = new ESMLoader();
 
   // Importation must be handled by internal loader to avoid poluting userland
-  const keyedExportsList = await internalEsmLoader.import(
-    customLoaders,
-    pathToFileURL(cwd).href,
-    ObjectCreate(null),
-  );
+  const parentURL = pathToFileURL(cwd).href;
+  const importAssertions = ObjectCreate(null);
+
+  const keyedExportsList = await PromiseAll(new SafeArrayIterator(
+    ArrayPrototypeMap(customLoaders, async (url) => {
+      const exports = await internalEsmLoader.import(url, parentURL,
+                                                     importAssertions);
+      return { exports, url };
+    })
+  ));
 
   // Hooks must then be added to external/public loader
   // (so they're triggered in userland)
diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js
index 4a8e51f694f7e6..102ce50cf77013 100644
--- a/lib/internal/process/execution.js
+++ b/lib/internal/process/execution.js
@@ -79,7 +79,7 @@ function evalScript(name, body, breakFirstLine, print) {
       [kVmBreakFirstLineSymbol]: !!breakFirstLine,
       importModuleDynamically(specifier, _, importAssertions) {
         const loader = asyncESM.esmLoader;
-        return loader.import(specifier, baseUrl, importAssertions);
+        return loader.import(specifier, baseUrl, importAssertions, true);
       }
     }));
   if (print) {
diff --git a/lib/repl.js b/lib/repl.js
index dd274a3bab1881..4a434f0dcc5d55 100644
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -465,7 +465,7 @@ function REPLServer(prompt,
               displayErrors: true,
               importModuleDynamically: (specifier, _, importAssertions) => {
                 return asyncESM.esmLoader.import(specifier, parentURL,
-                                                 importAssertions);
+                                                 importAssertions, true);
               }
             });
           } catch (fallbackError) {
@@ -509,7 +509,7 @@ function REPLServer(prompt,
             displayErrors: true,
             importModuleDynamically: (specifier, _, importAssertions) => {
               return asyncESM.esmLoader.import(specifier, parentURL,
-                                               importAssertions);
+                                               importAssertions, true);
             }
           });
         } catch (e) {

From 5ed3ea7716f9231236c3af956a0d96d2cc85ebc7 Mon Sep 17 00:00:00 2001
From: Guy Bedford <guybedford@gmail.com>
Date: Sat, 18 Jun 2022 18:18:04 -0700
Subject: [PATCH 2/2] lint fixes

---
 lib/internal/modules/esm/loader.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
index 8cbf79650ed72b..82262089eb6833 100644
--- a/lib/internal/modules/esm/loader.js
+++ b/lib/internal/modules/esm/loader.js
@@ -502,7 +502,7 @@ class ESMLoader {
       conditions: DEFAULT_CONDITIONS,
       dynamic,
       importAssertions: this.#hasLoaderHooks ?
-          ObjectAssign(ObjectCreate(null), importAssertions) : importAssertions,
+        ObjectAssign(ObjectCreate(null), importAssertions) : importAssertions,
       parentURL
     });
     await PromiseAll(new SafeArrayIterator(ArrayPrototypeMap(
@@ -787,7 +787,7 @@ class ESMLoader {
     const context = ObjectFreeze({
       conditions: DEFAULT_CONDITIONS,
       importAssertions: this.#hasLoaderHooks ?
-          ObjectAssign(ObjectCreate(null), importAssertions) : importAssertions,
+        ObjectAssign(ObjectCreate(null), importAssertions) : importAssertions,
       parentURL,
     });