fix: address bug when strict and async middleware used together (#2164)

jly36963 · web-flow · commit cbc2eb726efc · 2022-05-15T21:02:05.000-04:00
diff --git a/lib/command.ts b/lib/command.ts
@@ -364,34 +364,16 @@ export class CommandInstance {
     pc.push(c);
     return `$0 ${pc.join(' ')}`;
   }
-  private applyMiddlewareAndGetResult(
+  private handleValidationAndGetResult(
     isDefaultCommand: boolean,
     commandHandler: CommandHandler,
     innerArgv: Arguments | Promise<Arguments>,
     currentContext: Context,
-    helpOnly: boolean,
     aliases: Dictionary<string[]>,
-    yargs: YargsInstance
-  ): Arguments | Promise<Arguments> {
-    let positionalMap: Dictionary<string[]> = {};
-    // If showHelp() or getHelp() is being run, we should not
-    // execute middleware or handlers (these may perform expensive operations
-    // like creating a DB connection).
-    if (helpOnly) return innerArgv;
-    if (!yargs.getInternalMethods().getHasOutput()) {
-      positionalMap = this.populatePositionals(
-        commandHandler,
-        innerArgv as Arguments,
-        currentContext,
-        yargs
-      );
-    }
-    const middlewares = this.globalMiddleware
-      .getMiddleware()
-      .slice(0)
-      .concat(commandHandler.middlewares);
-    innerArgv = applyMiddleware(innerArgv, yargs, middlewares, true);
-
+    yargs: YargsInstance,
+    middlewares: Middleware[],
+    positionalMap: Dictionary<string[]>
+  ) {
     // we apply validation post-hoc, so that custom
     // checks get passed populated positional arguments.
     if (!yargs.getInternalMethods().getHasOutput()) {
@@ -453,6 +435,64 @@ export class CommandInstance {
 
     return innerArgv;
   }
+  private applyMiddlewareAndGetResult(
+    isDefaultCommand: boolean,
+    commandHandler: CommandHandler,
+    innerArgv: Arguments,
+    currentContext: Context,
+    helpOnly: boolean,
+    aliases: Dictionary<string[]>,
+    yargs: YargsInstance
+  ): Arguments | Promise<Arguments> {
+    let positionalMap: Dictionary<string[]> = {};
+    // If showHelp() or getHelp() is being run, we should not
+    // execute middleware or handlers (these may perform expensive operations
+    // like creating a DB connection).
+    if (helpOnly) return innerArgv;
+    if (!yargs.getInternalMethods().getHasOutput()) {
+      positionalMap = this.populatePositionals(
+        commandHandler,
+        innerArgv as Arguments,
+        currentContext,
+        yargs
+      );
+    }
+    const middlewares = this.globalMiddleware
+      .getMiddleware()
+      .slice(0)
+      .concat(commandHandler.middlewares);
+
+    const maybePromiseArgv = applyMiddleware(
+      innerArgv,
+      yargs,
+      middlewares,
+      true
+    );
+
+    return isPromise(maybePromiseArgv)
+      ? maybePromiseArgv.then(resolvedInnerArgv =>
+          this.handleValidationAndGetResult(
+            isDefaultCommand,
+            commandHandler,
+            resolvedInnerArgv,
+            currentContext,
+            aliases,
+            yargs,
+            middlewares,
+            positionalMap
+          )
+        )
+      : this.handleValidationAndGetResult(
+          isDefaultCommand,
+          commandHandler,
+          maybePromiseArgv,
+          currentContext,
+          aliases,
+          yargs,
+          middlewares,
+          positionalMap
+        );
+  }
   // transcribe all positional arguments "command <foo> <bar> [apple]"
   // onto argv.
   private populatePositionals(
diff --git a/test/middleware.cjs b/test/middleware.cjs
@@ -281,6 +281,26 @@ describe('middleware', () => {
       }
     });
 
+    // Addresses: https://github.com/yargs/yargs/issues/2124
+    // This test will fail if result of async middleware is not handled like a promise
+    it('does not cause an unexpected error when async middleware and strict are both used', done => {
+      const input = 'cmd1';
+      yargs(input)
+        .command(
+          'cmd1',
+          'cmd1 desc',
+          yargs => yargs.middleware(async argv => argv, true),
+          _argv => {
+            done();
+          }
+        )
+        .fail(msg => {
+          done(new Error(msg));
+        })
+        .strict()
+        .parse();
+    });
+
     it('runs before validation, when middleware is added in builder', done => {
       yargs(['mw'])
         .command(
