From a1aef893057f731b7f7ed327713521d6eb090565 Mon Sep 17 00:00:00 2001 From: Izaak Schroeder Date: Thu, 24 Aug 2023 13:07:24 -0700 Subject: [PATCH] =?UTF-8?q?esm:=20refactor=20hooks=20=E2=80=93=20add=20loa?= =?UTF-8?q?der=20instances?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously we had one array for each kind of chain in the variable `#chains`; this is probably the most efficient, but it decouples the connection between the original `register` call and the hook functions themselves. By adding `#loaderInstances` we can keep track of each time `register` is called. This allows, for example, trivially removing the loader later on, or associating shared context with a particular `register` call. The tradeoff here if we just did this would be that during hook traversal we may iterate over a slightly larger number of objects. Instead we keep `#hooks` but it is now a derived property; every time `#loaderInstances` changes we simply rebuild `#hooks`. --- lib/internal/modules/esm/default_loader.js | 7 ++ lib/internal/modules/esm/hooks.js | 99 ++++++++++------------ 2 files changed, 52 insertions(+), 54 deletions(-) create mode 100644 lib/internal/modules/esm/default_loader.js diff --git a/lib/internal/modules/esm/default_loader.js b/lib/internal/modules/esm/default_loader.js new file mode 100644 index 00000000000000..e6998ee18ce2aa --- /dev/null +++ b/lib/internal/modules/esm/default_loader.js @@ -0,0 +1,7 @@ +'use strict'; + +const { defaultLoad } = require('internal/modules/esm/load'); +const { defaultResolve } = require('internal/modules/esm/resolve'); + +exports.resolve = defaultResolve; +exports.load = defaultLoad; diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index b7e1afac31060d..771f4f2720183b 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -54,7 +54,6 @@ const { } = require('internal/util'); const { - defaultResolve, throwIfInvalidParentURL, } = require('internal/modules/esm/resolve'); const { @@ -87,45 +86,40 @@ let importMetaInitializer; // [2] `validate...()`s throw the wrong error class Hooks { - #chains = { - /** - * Prior to ESM loading. These are called once before any modules are started. - * @private - * @property {KeyedHook[]} globalPreload Last-in-first-out list of preload hooks. - */ - globalPreload: [], - - /** - * Phase 1 of 2 in ESM loading. - * The output of the `resolve` chain of hooks is passed into the `load` chain of hooks. - * @private - * @property {KeyedHook[]} resolve Last-in-first-out collection of resolve hooks. - */ - resolve: [ - { - fn: defaultResolve, - url: 'node:internal/modules/esm/resolve', - }, - ], - - /** - * Phase 2 of 2 in ESM loading. - * @private - * @property {KeyedHook[]} load Last-in-first-out collection of loader hooks. - */ - load: [ - { - fn: require('internal/modules/esm/load').defaultLoad, - url: 'node:internal/modules/esm/load', - }, - ], - }; + #loaderInstances = []; + #chains = {}; // Cache URLs we've already validated to avoid repeated validation #validatedUrls = new SafeSet(); allowImportMetaResolve = false; + constructor() { + const defaultLoader = 'internal/modules/esm/default_loader'; + this.addCustomLoader(`node:${defaultLoader}`, require(defaultLoader)); + } + + #rebuildChain(name) { + const chain = this.#chains[name] = []; + let i = 0; + for (const instance of this.#loaderInstances) { + if (typeof instance[name] !== 'function') { + continue; + } + chain.push({ + loader: instance, + fn: instance[name], + next: chain[i++ - 1], + }); + } + } + + #rebuildChains() { + this.#rebuildChain('globalPreload'); + this.#rebuildChain('resolve'); + this.#rebuildChain('load'); + } + /** * Import and register custom/user-defined module loader hook(s). * @param {string} urlOrSpecifier @@ -164,17 +158,18 @@ class Hooks { emitExperimentalWarning( '`globalPreload` is planned for removal in favor of `initialize`. `globalPreload`', ); - ArrayPrototypePush(this.#chains.globalPreload, { __proto__: null, fn: globalPreload, url }); } - if (resolve) { - const next = this.#chains.resolve[this.#chains.resolve.length - 1]; - ArrayPrototypePush(this.#chains.resolve, { __proto__: null, fn: resolve, url, next }); - } - if (load) { - const next = this.#chains.load[this.#chains.load.length - 1]; - ArrayPrototypePush(this.#chains.load, { __proto__: null, fn: load, url, next }); - } - return initialize?.(data); + const instance = { + __proto__: null, + url, + globalPreload, + initialize, + resolve, + load, + }; + ArrayPrototypePush(this.#loaderInstances, instance); + this.#rebuildChains(); + return initialize?.(data, { __proto__: null, id: instance.id, url }); } /** @@ -182,7 +177,7 @@ class Hooks { */ initializeGlobalPreload() { const preloadScripts = []; - for (let i = this.#chains.globalPreload.length - 1; i >= 0; i--) { + for (const chainEntry of this.#chains.globalPreload) { const { MessageChannel } = require('internal/worker/io'); const channel = new MessageChannel(); const { @@ -193,10 +188,8 @@ class Hooks { insidePreload.unref(); insideLoader.unref(); - const { - fn: preload, - url: specifier, - } = this.#chains.globalPreload[i]; + const preload = chainEntry.fn; + const specifier = chainEntry.loader.url; const preloaded = preload({ port: insideLoader, @@ -789,11 +782,9 @@ function pluckHooks({ function nextHookFactory(current, meta, { validateArgs, validateOutput }) { // First, prepare the current const { hookName } = meta; - const { - fn: hook, - url: hookFilePath, - next, - } = current; + + const { next, fn: hook, loader } = current; + const { url: hookFilePath } = loader; // ex 'nextResolve' const nextHookName = `next${