Skip to content

Commit

Permalink
esm: fix chain advances when loader calls next<HookName> multiple times
Browse files Browse the repository at this point in the history
PR-URL: #43303
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
JakobJingleheimer authored and targos committed Jul 19, 2022
1 parent 1bd62be commit 4c726b0
Show file tree
Hide file tree
Showing 6 changed files with 198 additions and 109 deletions.
2 changes: 1 addition & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1360,7 +1360,7 @@ E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe', Error);
E('ERR_IPC_SYNC_FORK', 'IPC cannot be used with synchronous forks', Error);
E(
'ERR_LOADER_CHAIN_INCOMPLETE',
'The "%s" hook from %s did not call the next hook in its chain and did not' +
'"%s" did not call the next hook in its chain and did not' +
' explicitly signal a short circuit. If this is intentional, include' +
' `shortCircuit: true` in the hook\'s return.',
Error
Expand Down
191 changes: 115 additions & 76 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,22 @@ const {
FunctionPrototypeCall,
ObjectAssign,
ObjectCreate,
ObjectDefineProperty,
ObjectSetPrototypeOf,
PromiseAll,
ReflectApply,
RegExpPrototypeExec,
SafeArrayIterator,
SafeWeakMap,
StringPrototypeSlice,
StringPrototypeToUpperCase,
globalThis,
} = primordials;
const { MessageChannel } = require('internal/worker/io');

const {
ERR_LOADER_CHAIN_INCOMPLETE,
ERR_INTERNAL_ASSERTION,
ERR_INVALID_ARG_TYPE,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_RETURN_PROPERTY_VALUE,
Expand Down Expand Up @@ -89,6 +94,81 @@ const { getOptionValue } = require('internal/options');

let emittedSpecifierResolutionWarning = false;

/**
* A utility function to iterate through a hook chain, track advancement in the
* chain, and generate and supply the `next<HookName>` argument to the custom
* hook.
* @param {KeyedHook[]} chain The whole hook chain.
* @param {object} meta Properties that change as the current hook advances
* along the chain.
* @param {boolean} meta.chainFinished Whether the end of the chain has been
* reached AND invoked.
* @param {string} meta.hookErrIdentifier A user-facing identifier to help
* pinpoint where an error occurred. Ex "file:///foo.mjs 'resolve'".
* @param {number} meta.hookIndex A non-negative integer tracking the current
* position in the hook chain.
* @param {string} meta.hookName The kind of hook the chain is (ex 'resolve')
* @param {boolean} meta.shortCircuited Whether a hook signaled a short-circuit.
* @param {(hookErrIdentifier, hookArgs) => void} validate A wrapper function
* containing all validation of a custom loader hook's intermediary output. Any
* validation within MUST throw.
* @returns {function next<HookName>(...hookArgs)} The next hook in the chain.
*/
function nextHookFactory(chain, meta, validate) {
// First, prepare the current
const { hookName } = meta;
const {
fn: hook,
url: hookFilePath,
} = chain[meta.hookIndex];

// ex 'nextResolve'
const nextHookName = `next${
StringPrototypeToUpperCase(hookName[0]) +
StringPrototypeSlice(hookName, 1)
}`;

// When hookIndex is 0, it's reached the default, which does not call next()
// so feed it a noop that blows up if called, so the problem is obvious.
const generatedHookIndex = meta.hookIndex;
let nextNextHook;
if (meta.hookIndex > 0) {
// Now, prepare the next: decrement the pointer so the next call to the
// factory generates the next link in the chain.
meta.hookIndex--;

nextNextHook = nextHookFactory(chain, meta, validate);
} else {
// eslint-disable-next-line func-name-matching
nextNextHook = function chainAdvancedTooFar() {
throw new ERR_INTERNAL_ASSERTION(
`ESM custom loader '${hookName}' advanced beyond the end of the chain.`
);
};
}

return ObjectDefineProperty(
async (...args) => {
// Update only when hook is invoked to avoid fingering the wrong filePath
meta.hookErrIdentifier = `${hookFilePath} '${hookName}'`;

validate(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, args);

// Set when next<HookName> is actually called, not just generated.
if (generatedHookIndex === 0) { meta.chainFinished = true; }

ArrayPrototypePush(args, nextNextHook);
const output = await ReflectApply(hook, undefined, args);

if (output?.shortCircuit === true) { meta.shortCircuited = true; }
return output;

},
'name',
{ __proto__: null, value: nextHookName },
);
}

/**
* An ESMLoader instance is used as the main entry point for loading ES modules.
* Currently, this is a singleton -- there is only one used for loading
Expand Down Expand Up @@ -471,32 +551,21 @@ class ESMLoader {
* @returns {{ format: ModuleFormat, source: ModuleSource }}
*/
async load(url, context = {}) {
const loaders = this.#loaders;
let hookIndex = loaders.length - 1;
let {
fn: loader,
url: loaderFilePath,
} = loaders[hookIndex];
let chainFinished = hookIndex === 0;
let shortCircuited = false;

const nextLoad = async (nextUrl, ctx = context) => {
--hookIndex; // `nextLoad` has been called, so decrement our pointer.

({
fn: loader,
url: loaderFilePath,
} = loaders[hookIndex]);

if (hookIndex === 0) { chainFinished = true; }

const hookErrIdentifier = `${loaderFilePath} "load"`;
const chain = this.#loaders;
const meta = {
chainFinished: null,
hookErrIdentifier: '',
hookIndex: chain.length - 1,
hookName: 'load',
shortCircuited: false,
};

const validate = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => {
if (typeof nextUrl !== 'string') {
// non-strings can be coerced to a url string
// validateString() throws a less-specific error
throw new ERR_INVALID_ARG_TYPE(
`${hookErrIdentifier} nextLoad(url)`,
`${hookErrIdentifier} url`,
'a url string',
nextUrl,
);
Expand All @@ -508,29 +577,20 @@ class ESMLoader {
new URL(nextUrl);
} catch {
throw new ERR_INVALID_ARG_VALUE(
`${hookErrIdentifier} nextLoad(url)`,
`${hookErrIdentifier} url`,
nextUrl,
'should be a url string',
);
}
}

validateObject(ctx, `${hookErrIdentifier} nextLoad(, context)`);

const output = await loader(nextUrl, ctx, nextLoad);

if (output?.shortCircuit === true) { shortCircuited = true; }

return output;
validateObject(ctx, `${hookErrIdentifier} context`);
};

const loaded = await loader(
url,
context,
nextLoad,
);
const nextLoad = nextHookFactory(chain, meta, validate);

const hookErrIdentifier = `${loaderFilePath} load`;
const loaded = await nextLoad(url, context);
const { hookErrIdentifier } = meta; // Retrieve the value after all settled

if (typeof loaded !== 'object') { // [2]
throw new ERR_INVALID_RETURN_VALUE(
Expand All @@ -540,10 +600,10 @@ class ESMLoader {
);
}

if (loaded?.shortCircuit === true) { shortCircuited = true; }
if (loaded?.shortCircuit === true) { meta.shortCircuited = true; }

if (!chainFinished && !shortCircuited) {
throw new ERR_LOADER_CHAIN_INCOMPLETE('load', loaderFilePath);
if (!meta.chainFinished && !meta.shortCircuited) {
throw new ERR_LOADER_CHAIN_INCOMPLETE(hookErrIdentifier);
}

const {
Expand Down Expand Up @@ -735,55 +795,34 @@ class ESMLoader {
parentURL,
);
}
const resolvers = this.#resolvers;

let hookIndex = resolvers.length - 1;
let {
fn: resolver,
url: resolverFilePath,
} = resolvers[hookIndex];
let chainFinished = hookIndex === 0;
let shortCircuited = false;
const chain = this.#resolvers;
const meta = {
chainFinished: null,
hookErrIdentifier: '',
hookIndex: chain.length - 1,
hookName: 'resolve',
shortCircuited: false,
};

const context = {
conditions: DEFAULT_CONDITIONS,
importAssertions,
parentURL,
};

const nextResolve = async (suppliedSpecifier, ctx = context) => {
--hookIndex; // `nextResolve` has been called, so decrement our pointer.

({
fn: resolver,
url: resolverFilePath,
} = resolvers[hookIndex]);

if (hookIndex === 0) { chainFinished = true; }

const hookErrIdentifier = `${resolverFilePath} "resolve"`;
const validate = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {

validateString(
suppliedSpecifier,
`${hookErrIdentifier} nextResolve(specifier)`,
`${hookErrIdentifier} specifier`,
); // non-strings can be coerced to a url string

validateObject(ctx, `${hookErrIdentifier} nextResolve(, context)`);

const output = await resolver(suppliedSpecifier, ctx, nextResolve);

if (output?.shortCircuit === true) { shortCircuited = true; }

return output;
validateObject(ctx, `${hookErrIdentifier} context`);
};

const resolution = await resolver(
originalSpecifier,
context,
nextResolve,
);
const nextResolve = nextHookFactory(chain, meta, validate);

const hookErrIdentifier = `${resolverFilePath} resolve`;
const resolution = await nextResolve(originalSpecifier, context);
const { hookErrIdentifier } = meta; // Retrieve the value after all settled

if (typeof resolution !== 'object') { // [2]
throw new ERR_INVALID_RETURN_VALUE(
Expand All @@ -793,10 +832,10 @@ class ESMLoader {
);
}

if (resolution?.shortCircuit === true) { shortCircuited = true; }
if (resolution?.shortCircuit === true) { meta.shortCircuited = true; }

if (!chainFinished && !shortCircuited) {
throw new ERR_LOADER_CHAIN_INCOMPLETE('resolve', resolverFilePath);
if (!meta.chainFinished && !meta.shortCircuited) {
throw new ERR_LOADER_CHAIN_INCOMPLETE(hookErrIdentifier);
}

const {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -1115,7 +1115,7 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) {
}
}

async function defaultResolve(specifier, context = {}, defaultResolveUnused) {
async function defaultResolve(specifier, context = {}) {
let { parentURL, conditions } = context;
if (parentURL && policy?.manifest) {
const redirects = policy.manifest.getDependencyMapper(parentURL);
Expand Down
Loading

0 comments on commit 4c726b0

Please sign in to comment.