From ab935b6fda8a6522634ebc66106eabfc7320ca69 Mon Sep 17 00:00:00 2001 From: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com> Date: Sun, 12 Jun 2022 15:50:25 +0200 Subject: [PATCH] fixup: change async/thenable detection --- lib/internal/modules/esm/loader.js | 85 +++++++++++++++--------------- 1 file changed, 43 insertions(+), 42 deletions(-) diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 6897c0e9bbfae2..da43474a5724c2 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -15,6 +15,8 @@ const { ObjectDefineProperty, ObjectSetPrototypeOf, PromiseAll, + PromiseResolve, + PromisePrototypeThen, ReflectApply, RegExpPrototypeExec, SafeArrayIterator, @@ -109,12 +111,14 @@ let emittedSpecifierResolutionWarning = false; * 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 + * @param {object} validators A wrapper function * containing all validation of a custom loader hook's intermediary output. Any * validation within MUST throw. + * @param {(hookErrIdentifier, hookArgs) => void} validateArgs + * @param {(hookErrIdentifier, output) => void} validateOutput * @returns {function next(...hookArgs)} The next hook in the chain. */ -function nextHookFactory(chain, meta, validate) { +function nextHookFactory(chain, meta, { validateArgs, validateOutput }) { // First, prepare the current const { hookName } = meta; const { @@ -137,7 +141,7 @@ function nextHookFactory(chain, meta, validate) { // factory generates the next link in the chain. meta.hookIndex--; - nextNextHook = nextHookFactory(chain, meta, validate); + nextNextHook = nextHookFactory(chain, meta, { validateArgs, validateOutput }); } else { // eslint-disable-next-line func-name-matching nextNextHook = function chainAdvancedTooFar() { @@ -152,34 +156,25 @@ function nextHookFactory(chain, meta, validate) { // Update only when hook is invoked to avoid fingering the wrong filePath meta.hookErrIdentifier = `${hookFilePath} '${hookName}'`; - validate(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, args); + validateArgs(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, args); + + const outputErrIdentifier = `${chain[generatedHookIndex].url} '${hookName}' hook's ${nextHookName}()`; // Set when next is actually called, not just generated. if (generatedHookIndex === 0) { meta.chainFinished = true; } ArrayPrototypePush(args, nextNextHook); - const output = ReflectApply(hook, undefined, args); + let output = ReflectApply(hook, undefined, args); + + validateOutput(outputErrIdentifier, output); function checkShortCircuited(output) { if (output?.shortCircuit === true) { meta.shortCircuited = true; } } - const then = output?.then; - if (typeof then === 'function') { - if (!meta.isChainAsync) { - throw ERR_INVALID_RETURN_VALUE( - 'an object', - // MUST use generatedHookIndex because the chain has already advanced, - // causing meta.hookIndex to advance - `${chain[generatedHookIndex].url} '${hookName}' hook's ${nextHookName}()`, - output, - ); - } - - ReflectApply(then, output, [ - checkShortCircuited, - // TODO: handle error case - ]); + if (meta.isChainAsync) { + output = PromiseResolve(output); + PromisePrototypeThen(output, checkShortCircuited); } else { checkShortCircuited(output); } @@ -586,7 +581,7 @@ class ESMLoader { shortCircuited: false, }; - const validate = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => { + const validateArgs = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => { if (typeof nextUrl !== 'string') { // non-strings can be coerced to a url string // validateString() throws a less-specific error @@ -612,19 +607,22 @@ class ESMLoader { validateObject(ctx, `${hookErrIdentifier} context`); }; + const validateOutput = (hookErrIdentifier, output) => { + if (typeof output !== 'object') { // [2] + throw new ERR_INVALID_RETURN_VALUE( + 'an object', + hookErrIdentifier, + output, + ); + } + }; - const nextLoad = nextHookFactory(chain, meta, validate); + const nextLoad = nextHookFactory(chain, meta, { validateArgs, validateOutput }); 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( - 'an object', - hookErrIdentifier, - loaded, - ); - } + validateOutput(hookErrIdentifier, loaded); if (loaded?.shortCircuit === true) { meta.shortCircuited = true; } @@ -837,7 +835,7 @@ class ESMLoader { parentURL, }; - const validate = (hookErrIdentifier, output) => { + const validateArgs = (hookErrIdentifier, output) => { const { 0: suppliedSpecifier, 1: ctx } = output; validateString( @@ -847,22 +845,25 @@ class ESMLoader { validateObject(ctx, `${hookErrIdentifier} context`); }; + const validateOutput = (hookErrIdentifier, output) => { + if ( + typeof output !== 'object' || // [2] + typeof output.then === 'function' + ) { + throw new ERR_INVALID_RETURN_VALUE( + 'an object', + hookErrIdentifier, + output, + ); + } + }; - const nextResolve = nextHookFactory(chain, meta, validate); + const nextResolve = nextHookFactory(chain, meta, { validateArgs, validateOutput }); const resolution = nextResolve(originalSpecifier, context); const { hookErrIdentifier } = meta; // Retrieve the value after all settled - if ( - typeof resolution !== 'object' || // [2] - typeof resolution?.then === 'function' - ) { - throw new ERR_INVALID_RETURN_VALUE( - 'an object', - hookErrIdentifier, - resolution, - ); - } + validateOutput(hookErrIdentifier, resolution); if (resolution?.shortCircuit === true) { meta.shortCircuited = true; }