Skip to content

Commit

Permalink
esm: remove support for arrays in import internal method
Browse files Browse the repository at this point in the history
This avoids initializing arrays that we never use, and simplifies the
implementation overall.

PR-URL: nodejs#48296
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
aduh95 authored and targos committed Nov 10, 2023
1 parent 09c986e commit 96edd85
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 76 deletions.
42 changes: 16 additions & 26 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<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 });
}
}

Expand Down
46 changes: 5 additions & 41 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@
require('internal/modules/cjs/loader');

const {
Array,
ArrayIsArray,
FunctionPrototypeCall,
ObjectSetPrototypeOf,
PromisePrototypeThen,
SafePromiseAllReturnArrayLike,
SafeWeakMap,
} = primordials;

Expand Down Expand Up @@ -246,51 +243,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();
}

/**
Expand Down
12 changes: 6 additions & 6 deletions lib/internal/modules/esm/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ function isLoaderWorker() {
}

async function initializeHooks() {
const customLoaderPaths = getOptionValue('--experimental-loader');
const customLoaderURLs = getOptionValue('--experimental-loader');

let cwd;
try {
Expand Down Expand Up @@ -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();
Expand Down
10 changes: 7 additions & 3 deletions lib/internal/process/esm_loader.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
'use strict';

const {
SafePromiseAllReturnVoid,
} = primordials;

const { createModuleLoader } = require('internal/modules/esm/loader');
const { getOptionValue } = require('internal/options');
const {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 96edd85

Please sign in to comment.