esm: remove support for arrays in import internal method

This avoids initializing arrays that we never use, and simplifies the implementation overall. PR-URL: #48296 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jun 5, 2023 · 11a3434 · 11a3434
1 parent 44411fc
commit 11a3434
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 76 deletions.
diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js
@@ -110,37 +110,27 @@ class Hooks {
   // Cache URLs we've already validated to avoid repeated validation
   #validatedUrls = new SafeSet();
 
-  constructor(userLoaders) {
-    this.addCustomLoaders(userLoaders);
-  }
-
   /**
    * Collect custom/user-defined module loader hook(s).
    * After all hooks have been collected, the global preload hook(s) must be initialized.
-   * @param {import('./loader.js).KeyedExports} customLoaders Exports from user-defined loaders
-   *  (as returned by `ModuleLoader.import()`).
+   * @param {string} url Custom loader specifier
+   * @param {Record<string, unknown>} exports
    */
-  addCustomLoaders(customLoaders = []) {
-    for (let i = 0; i < customLoaders.length; i++) {
-      const {
-        exports,
-        url,
-      } = customLoaders[i];
-      const {
-        globalPreload,
-        resolve,
-        load,
-      } = pluckHooks(exports);
+  addCustomLoader(url, exports) {
+    const {
+      globalPreload,
+      resolve,
+      load,
+    } = pluckHooks(exports);
 
-      if (globalPreload) {
-        ArrayPrototypePush(this.#chains.globalPreload, { fn: globalPreload, url });
-      }
-      if (resolve) {
-        ArrayPrototypePush(this.#chains.resolve, { fn: resolve, url });
-      }
-      if (load) {
-        ArrayPrototypePush(this.#chains.load, { fn: load, url });
-      }
+    if (globalPreload) {
+      ArrayPrototypePush(this.#chains.globalPreload, { fn: globalPreload, url });
+    }
+    if (resolve) {
+      ArrayPrototypePush(this.#chains.resolve, { fn: resolve, url });
+    }
+    if (load) {
+      ArrayPrototypePush(this.#chains.load, { fn: load, url });
     }
   }
 

diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js
@@ -4,12 +4,9 @@
 require('internal/modules/cjs/loader');
 
 const {
-  Array,
-  ArrayIsArray,
   FunctionPrototypeCall,
   ObjectSetPrototypeOf,
   PromisePrototypeThen,
-  SafePromiseAllReturnArrayLike,
   SafeWeakMap,
 } = primordials;
 
@@ -236,51 +233,18 @@ class DefaultModuleLoader {
    *
    * 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} specifier The first parameter of an `import()` expression.
    * @param {string} parentURL Path 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).
    */
-  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] = PromisePrototypeThen(
-        this
-          .getModuleJob(specifiers[i], parentURL, importAssertions)
-          .run(),
-        ({ module }) => module.getNamespace(),
-      );
-    }
-
-    const namespaces = await SafePromiseAllReturnArrayLike(jobs);
-
-    if (!wasArr) { return namespaces[0]; } // We can skip the pairing below
-
-    for (let i = 0; i < count; i++) {
-      namespaces[i] = {
-        __proto__: null,
-        url: specifiers[i],
-        exports: namespaces[i],
-      };
-    }
-
-    return namespaces;
+  async import(specifier, parentURL, importAssertions) {
+    const moduleJob = this.getModuleJob(specifier, parentURL, importAssertions);
+    const { module } = await moduleJob.run();
+    return module.getNamespace();
   }
 
   /**

diff --git a/lib/internal/modules/esm/utils.js b/lib/internal/modules/esm/utils.js
@@ -108,7 +108,7 @@ function isLoaderWorker() {
 }
 
 async function initializeHooks() {
-  const customLoaderPaths = getOptionValue('--experimental-loader');
+  const customLoaderURLs = getOptionValue('--experimental-loader');
 
   let cwd;
   try {
@@ -149,17 +149,17 @@ async function initializeHooks() {
 
   const parentURL = pathToFileURL(cwd).href;
 
-  for (let i = 0; i < customLoaderPaths.length; i++) {
-    const customLoaderPath = customLoaderPaths[i];
+  for (let i = 0; i < customLoaderURLs.length; i++) {
+    const customLoaderURL = customLoaderURLs[i];
 
     // Importation must be handled by internal loader to avoid polluting user-land
-    const keyedExportsSublist = await privateModuleLoader.import(
-      [customLoaderPath], // Import can handle multiple paths, but custom loaders must be sequential
+    const keyedExports = await privateModuleLoader.import(
+      customLoaderURL,
       parentURL,
       kEmptyObject,
     );
 
-    hooks.addCustomLoaders(keyedExportsSublist);
+    hooks.addCustomLoader(customLoaderURL, keyedExports);
   }
 
   const preloadScripts = hooks.initializeGlobalPreload();

diff --git a/lib/internal/process/esm_loader.js b/lib/internal/process/esm_loader.js
@@ -1,5 +1,9 @@
 'use strict';
 
+const {
+  SafePromiseAllReturnVoid,
+} = primordials;
+
 const { createModuleLoader } = require('internal/modules/esm/loader');
 const { getOptionValue } = require('internal/options');
 const {
@@ -27,11 +31,11 @@ module.exports = {
           cwd = '/';
         }
         const parentURL = pathToFileURL(cwd).href;
-        await esmLoader.import(
-          userImports,
+        await SafePromiseAllReturnVoid(userImports, (specifier) => esmLoader.import(
+          specifier,
           parentURL,
           kEmptyObject,
-        );
+        ));
       }
       await callback(esmLoader);
     } catch (err) {