fixup: change async/thenable detection

Author: JakobJingleheimer

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<HookName>(...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<HookName> 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; }