From 1c7be606f1e99f433b04e9ac8c934ade85deeb79 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Mon, 5 Jun 2023 10:32:33 +0200 Subject: [PATCH] esm: remove support for arrays in `import` internal method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids initializing arrays that we never use, and simplifies the implementation overall. PR-URL: https://github.com/nodejs/node/pull/48296 Backport-PR-URL: https://github.com/nodejs/node/pull/50669 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Michaël Zasso Reviewed-By: Jacob Smith Reviewed-By: Geoffrey Booth Reviewed-By: James M Snell --- lib/internal/modules/esm/hooks.js | 42 +++++++++++---------------- lib/internal/modules/esm/loader.js | 46 ++++-------------------------- lib/internal/modules/esm/utils.js | 12 ++++---- lib/internal/process/esm_loader.js | 10 +++++-- 4 files changed, 34 insertions(+), 76 deletions(-) diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 982a5c6dbf3138..3a01f7ffb71014 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -109,37 +109,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} 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 index fef7f8cb579fef..2c48702a3816ff 100644 --- 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; @@ -246,7 +243,7 @@ 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} importAssertions Validations for the * module import. @@ -254,43 +251,10 @@ class DefaultModuleLoader { * 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 index a615d4ecbbbdcf..0fd833b6544945 100644 --- 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 index e7d70ebbca1ca4..9a84ed944e87c4 100644 --- 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) {