Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esm: remove support for arrays in import internal method #48296

Merged
merged 2 commits into from
Jun 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 16 additions & 26 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
}

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 @@ -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();
}

/**
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