feat(async): add support for async check and coerce (#1872)

BREAKING CHANGE: yargs now returns a promise if async or check are asynchronous.
refactor(coerce)!: coerce is now applied before validation.
refactor(check): check callback is no longer passed an aliases object.

docs/api.md

@@ -115,7 +115,7 @@ If `key` is an array, interpret all the elements as booleans.
 
 Check that certain conditions are met in the provided arguments.
 
-`fn` is called with two arguments, the parsed `argv` hash and an array of options and their aliases.
+`fn` is called with the parsed `argv` hash.
 
 If `fn` throws or returns a non-truthy value, Yargs will show the thrown error
 and usage information. Yargs will then exit, unless
@@ -175,7 +175,7 @@ var argv = require('yargs/yargs')(process.argv.slice(2))
 <a name="coerce"></a>.coerce(key, fn)
 ----------------
 
-Provide a synchronous function to coerce or transform the value(s) given on the
+Provide a function to coerce or transform the value(s) given on the
 command line for `key`.
 
 The coercion function should accept one argument, representing the parsed value from
@@ -184,8 +184,7 @@ return a new value or throw an error. The returned value will be used as the val
 `key` (or one of its aliases) in `argv`.
 
 If the function throws, the error will be treated as a validation
-failure, delegating to either a custom [`.fail()`](#fail) handler or printing
-the error message in the console.
+failure, delegating to either a custom [`.fail()`](#fail) handler or printing the error message in the console.
 
 Coercion will be applied to a value after
 all other modifications, such as [`.normalize()`](#normalize).
@@ -195,7 +194,7 @@ _Examples:_
 ```js
 var argv = require('yargs/yargs')(process.argv.slice(2))
   .coerce('file', function (arg) {
-    return require('fs').readFileSync(arg, 'utf8')
+    return await require('fs').promises.readFile(arg, 'utf8')
   })
   .argv
 ```
@@ -212,8 +211,7 @@ var argv = require('yargs/yargs')(process.argv.slice(2))
   .argv
 ```
 
-You can also map the same function to several keys at one time. Just pass an
-array of keys as the first argument to `.coerce()`:
+You can also map the same function to several keys at one time. Just pass an array of keys as the first argument to `.coerce()`:
 
 ```js
 var path = require('path')
@@ -222,8 +220,7 @@ var argv = require('yargs/yargs')(process.argv.slice(2))
   .argv
 ```
 
-If you are using dot-notion or arrays, .e.g., `user.email` and `user.password`,
-coercion will be applied to the final object that has been parsed:
+If you are using dot-notion or arrays, .e.g., `user.email` and `user.password`, coercion will be applied to the final object that has been parsed:
 
 ```js
 // --user.name Batman --user.password 123

lib/cjs.ts

@@ -5,7 +5,6 @@ import {applyExtends} from './utils/apply-extends';
 import {argsert} from './argsert.js';
 import {isPromise} from './utils/is-promise.js';
 import {objFilter} from './utils/obj-filter.js';
-import {globalMiddlewareFactory} from './middleware.js';
 import {parseCommand} from './parse-command.js';
 import * as processArgv from './utils/process-argv.js';
 import {YargsWithShim, rebase} from './yargs-factory.js';
@@ -35,7 +34,6 @@ export default {
   cjsPlatformShim,
   Yargs,
   argsert,
-  globalMiddlewareFactory,
   isPromise,
   objFilter,
   parseCommand,

lib/command.ts

@@ -8,6 +8,7 @@ import {isPromise} from './utils/is-promise.js';
 import {
   applyMiddleware,
   commandMiddlewareFactory,
+  GlobalMiddleware,
   Middleware,
 } from './middleware.js';
 import {parseCommand, Positional} from './parse-command.js';
@@ -23,6 +24,7 @@ import {
   Arguments,
   DetailedArguments,
 } from './yargs-factory.js';
+import {maybeAsyncResult} from './utils/maybe-async-result.js';
 import whichModule from './utils/which-module.js';
 
 const DEFAULT_MARKER = /(^\*)|(^\$0)/;
@@ -35,7 +37,7 @@ export function command(
   yargs: YargsInstance,
   usage: UsageInstance,
   validation: ValidationInstance,
-  globalMiddleware: Middleware[] = [],
+  globalMiddleware: GlobalMiddleware,
   shim: PlatformShim
 ) {
   const self: CommandInstance = {} as CommandInstance;
@@ -307,6 +309,7 @@ export function command(
     if (helpOnly) return innerArgv;
 
     const middlewares = globalMiddleware
+      .getMiddleware()
       .slice(0)
       .concat(commandHandler.middlewares);
     innerArgv = applyMiddleware(innerArgv, yargs, middlewares, true);
@@ -320,16 +323,10 @@ export function command(
         (yargs.parsed as DetailedArguments).error,
         !command
       );
-      if (isPromise(innerArgv)) {
-        // If the middlware returned a promise, resolve the middleware
-        // before applying the validation:
-        innerArgv = innerArgv.then(argv => {
-          validation(argv);
-          return argv;
-        });
-      } else {
-        validation(innerArgv);
-      }
+      innerArgv = maybeAsyncResult<Arguments>(innerArgv, result => {
+        validation(result);
+        return result;
+      });
     }
 
     if (commandHandler.handler && !yargs._hasOutput()) {
@@ -342,18 +339,14 @@ export function command(
       yargs._postProcess(innerArgv, populateDoubleDash, false, false);
 
       innerArgv = applyMiddleware(innerArgv, yargs, middlewares, false);
-      if (isPromise(innerArgv)) {
-        const innerArgvRef = innerArgv;
-        innerArgv = innerArgv
-          .then(argv => commandHandler.handler(argv))
-          .then(() => innerArgvRef);
-      } else {
-        const handlerResult = commandHandler.handler(innerArgv);
+      innerArgv = maybeAsyncResult<Arguments>(innerArgv, result => {
+        const handlerResult = commandHandler.handler(result as Arguments);
         if (isPromise(handlerResult)) {
-          const innerArgvRef = innerArgv;
-          innerArgv = handlerResult.then(() => innerArgvRef);
+          return handlerResult.then(() => result);
+        } else {
+          return result;
         }
-      }
+      });
 
       yargs.getUsageInstance().cacheHelpMessage();
       if (isPromise(innerArgv) && !yargs._hasParseCallback()) {
@@ -499,8 +492,9 @@ export function command(
     if (!unparsed.length) return;
 
     const config: Configuration = Object.assign({}, options.configuration, {
-      'populate--': true,
+      'populate--': false,
     });
+
     const parsed = shim.Parser.detailed(
       unparsed,
       Object.assign({}, options, {

lib/middleware.ts

@@ -2,35 +2,72 @@ import {argsert} from './argsert.js';
 import {isPromise} from './utils/is-promise.js';
 import {YargsInstance, Arguments} from './yargs-factory.js';
 
-export function globalMiddlewareFactory<T>(
-  globalMiddleware: Middleware[],
-  context: T
-) {
-  return function (
+export class GlobalMiddleware {
+  globalMiddleware: Middleware[] = [];
+  yargs: YargsInstance;
+  frozens: Array<Middleware[]> = [];
+  constructor(yargs: YargsInstance) {
+    this.yargs = yargs;
+  }
+  addMiddleware(
     callback: MiddlewareCallback | MiddlewareCallback[],
-    applyBeforeValidation = false
-  ) {
+    applyBeforeValidation: boolean,
+    global = true
+  ): YargsInstance {
     argsert(
-      '<array|function> [boolean]',
-      [callback, applyBeforeValidation],
+      '<array|function> [boolean] [boolean]',
+      [callback, applyBeforeValidation, global],
       arguments.length
     );
     if (Array.isArray(callback)) {
       for (let i = 0; i < callback.length; i++) {
         if (typeof callback[i] !== 'function') {
           throw Error('middleware must be a function');
         }
-        (callback[
-          i
-        ] as Middleware).applyBeforeValidation = applyBeforeValidation;
+        const m = callback[i] as Middleware;
+        m.applyBeforeValidation = applyBeforeValidation;
+        m.global = global;
       }
-      Array.prototype.push.apply(globalMiddleware, callback as Middleware[]);
+      Array.prototype.push.apply(
+        this.globalMiddleware,
+        callback as Middleware[]
+      );
     } else if (typeof callback === 'function') {
-      (callback as Middleware).applyBeforeValidation = applyBeforeValidation;
-      globalMiddleware.push(callback as Middleware);
+      const m = callback as Middleware;
+      m.applyBeforeValidation = applyBeforeValidation;
+      m.global = global;
+      this.globalMiddleware.push(callback as Middleware);
     }
-    return context;
-  };
+    return this.yargs;
+  }
+  // For "coerce" middleware, only one middleware instance can be registered
+  // per option:
+  addCoerceMiddleware(
+    callback: MiddlewareCallback,
+    option: string
+  ): YargsInstance {
+    const aliases = this.yargs.getAliases();
+    this.globalMiddleware = this.globalMiddleware.filter(m => {
+      const toCheck = [...(aliases[option] ? aliases[option] : []), option];
+      if (!m.option) return true;
+      else return !toCheck.includes(m.option);
+    });
+    (callback as Middleware).option = option;
+    return this.addMiddleware(callback, true, true);
+  }
+  getMiddleware() {
+    return this.globalMiddleware;
+  }
+  freeze() {
+    this.frozens.push([...this.globalMiddleware]);
+  }
+  unfreeze() {
+    const frozen = this.frozens.pop();
+    if (frozen !== undefined) this.globalMiddleware = frozen;
+  }
+  reset() {
+    this.globalMiddleware = this.globalMiddleware.filter(m => m.global);
+  }
 }
 
 export function commandMiddlewareFactory(
@@ -85,4 +122,6 @@ export interface MiddlewareCallback {
 
 export interface Middleware extends MiddlewareCallback {
   applyBeforeValidation: boolean;
+  global: boolean;
+  option?: string;
 }

lib/utils/maybe-async-result.ts

@@ -0,0 +1,32 @@
+// maybeAsyncResult() allows the same error/completion handler to be
+// applied to a value regardless of whether it is a concrete value or an
+// eventual value.
+//
+// As of yargs@v17, if no asynchronous steps are run, .e.g, a
+// check() script that resolves a promise, yargs will return a concrete
+// value. If any asynchronous steps are introduced, yargs resolves a promise.
+import {isPromise} from './is-promise.js';
+export function maybeAsyncResult<T>(
+  getResult: (() => T | Promise<T>) | T | Promise<T>,
+  resultHandler: (result: T) => T | Promise<T>,
+  errorHandler: (err: Error) => T = (err: Error) => {
+    throw err;
+  }
+): T | Promise<T> {
+  try {
+    const result = isFunction(getResult) ? getResult() : getResult;
+    if (isPromise(result)) {
+      return result.then((result: T) => {
+        return resultHandler(result);
+      });
+    } else {
+      return resultHandler(result);
+    }
+  } catch (err) {
+    return errorHandler(err);
+  }
+}
+
+function isFunction(arg: (() => any) | any): arg is () => any {
+  return typeof arg === 'function';
+}

lib/validation.ts

@@ -14,7 +14,7 @@ import {DetailedArguments} from './typings/yargs-parser-types.js';
 const specialKeys = ['$0', '--', '_'];
 
 // validation-type-stuff, missing params,
-// bad implications, custom checks.
+// bad implications:
 export function validation(
   yargs: YargsInstance,
   usage: UsageInstance,
@@ -275,34 +275,6 @@ export function validation(
     usage.fail(msg);
   };
 
-  // custom checks, added using the `check` option on yargs.
-  let checks: CustomCheck[] = [];
-  self.check = function check(f, global) {
-    checks.push({
-      func: f,
-      global,
-    });
-  };
-
-  self.customChecks = function customChecks(argv, aliases) {
-    for (let i = 0, f; (f = checks[i]) !== undefined; i++) {
-      const func = f.func;
-      let result = null;
-      try {
-        result = func(argv, aliases);
-      } catch (err) {
-        usage.fail(err.message ? err.message : err, err);
-        continue;
-      }
-
-      if (!result) {
-        usage.fail(__('Argument check failed: %s', func.toString()));
-      } else if (typeof result === 'string' || result instanceof Error) {
-        usage.fail(result.toString(), result);
-      }
-    }
-  };
-
   // check implications, argument foo implies => argument bar.
   let implied: Dictionary<KeyOrPos[]> = {};
   self.implies = function implies(key, value) {
@@ -441,36 +413,32 @@ export function validation(
   self.reset = function reset(localLookup) {
     implied = objFilter(implied, k => !localLookup[k]);
     conflicting = objFilter(conflicting, k => !localLookup[k]);
-    checks = checks.filter(c => c.global);
     return self;
   };
 
   const frozens: FrozenValidationInstance[] = [];
   self.freeze = function freeze() {
     frozens.push({
       implied,
-      checks,
       conflicting,
     });
   };
   self.unfreeze = function unfreeze() {
     const frozen = frozens.pop();
     assertNotStrictEqual(frozen, undefined, shim);
-    ({implied, checks, conflicting} = frozen);
+    ({implied, conflicting} = frozen);
   };
 
   return self;
 }
 
 /** Instance of the validation module. */
 export interface ValidationInstance {
-  check(f: CustomCheck['func'], global: boolean): void;
   conflicting(argv: Arguments): void;
   conflicts(
     key: string | Dictionary<string | string[]>,
     value?: string | string[]
   ): void;
-  customChecks(argv: Arguments, aliases: DetailedArguments['aliases']): void;
   freeze(): void;
   getConflicting(): Dictionary<(string | undefined)[]>;
   getImplied(): Dictionary<KeyOrPos[]>;
@@ -503,14 +471,8 @@ export interface ValidationInstance {
   unknownCommands(argv: Arguments): boolean;
 }
 
-interface CustomCheck {
-  func: (argv: Arguments, aliases: DetailedArguments['aliases']) => any;
-  global: boolean;
-}
-
 interface FrozenValidationInstance {
   implied: Dictionary<KeyOrPos[]>;
-  checks: CustomCheck[];
   conflicting: Dictionary<(string | undefined)[]>;
 }