From fe083174950e0708dcd411ca30eda4bbe2333c40 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 20 Feb 2021 14:36:34 -0800 Subject: [PATCH 1/8] feat(async): add support for async check and coerce --- lib/cjs.ts | 2 - lib/command.ts | 4 +- lib/middleware.ts | 57 +++++++++++++++++++-------- lib/validation.ts | 42 +------------------- lib/yargs-factory.ts | 92 +++++++++++++++++++++++++++++++++++++------- package.json | 4 +- test/command.cjs | 23 +++++++++++ test/middleware.cjs | 82 +++++++++++++++++++++++++++------------ test/usage.cjs | 4 +- 9 files changed, 209 insertions(+), 101 deletions(-) diff --git a/lib/cjs.ts b/lib/cjs.ts index 3940c29e7..64b57f8ba 100644 --- a/lib/cjs.ts +++ b/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, diff --git a/lib/command.ts b/lib/command.ts index 81a7b50e5..116c5c617 100644 --- a/lib/command.ts +++ b/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'; @@ -35,7 +36,7 @@ export function command( yargs: YargsInstance, usage: UsageInstance, validation: ValidationInstance, - globalMiddleware: Middleware[] = [], + globalMiddleware: GlobalMiddleware, shim: PlatformShim ) { const self: CommandInstance = {} as CommandInstance; @@ -299,6 +300,7 @@ export function command( if (helpOnly) return innerArgv; const middlewares = globalMiddleware + .getMiddleware() .slice(0) .concat(commandHandler.middlewares); innerArgv = applyMiddleware(innerArgv, yargs, middlewares, true); diff --git a/lib/middleware.ts b/lib/middleware.ts index 7028d055b..70594185d 100644 --- a/lib/middleware.ts +++ b/lib/middleware.ts @@ -2,17 +2,21 @@ import {argsert} from './argsert.js'; import {isPromise} from './utils/is-promise.js'; import {YargsInstance, Arguments} from './yargs-factory.js'; -export function globalMiddlewareFactory( - globalMiddleware: Middleware[], - context: T -) { - return function ( +export class GlobalMiddleware { + globalMiddleware: Middleware[] = []; + yargs: YargsInstance; + frozens: Array = []; + constructor(yargs: YargsInstance) { + this.yargs = yargs; + } + addMiddleware( callback: MiddlewareCallback | MiddlewareCallback[], - applyBeforeValidation = false - ) { + applyBeforeValidation: boolean, + global = true + ): YargsInstance { argsert( - ' [boolean]', - [callback, applyBeforeValidation], + ' [boolean] [boolean]', + [callback, applyBeforeValidation, global], arguments.length ); if (Array.isArray(callback)) { @@ -20,17 +24,35 @@ export function globalMiddlewareFactory( 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; + } + 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 +107,5 @@ export interface MiddlewareCallback { export interface Middleware extends MiddlewareCallback { applyBeforeValidation: boolean; + global: boolean; } diff --git a/lib/validation.ts b/lib/validation.ts index 371374d46..871d4784a 100644 --- a/lib/validation.ts +++ b/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 = {}; self.implies = function implies(key, value) { @@ -441,7 +413,6 @@ 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; }; @@ -449,14 +420,13 @@ export function validation( 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; @@ -464,13 +434,11 @@ export function validation( /** Instance of the validation module. */ export interface ValidationInstance { - check(f: CustomCheck['func'], global: boolean): void; conflicting(argv: Arguments): void; conflicts( key: string | Dictionary, value?: string | string[] ): void; - customChecks(argv: Arguments, aliases: DetailedArguments['aliases']): void; freeze(): void; getConflicting(): Dictionary<(string | undefined)[]>; getImplied(): Dictionary; @@ -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; - checks: CustomCheck[]; conflicting: Dictionary<(string | undefined)[]>; } diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 69e79d663..cda8c323f 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -53,7 +53,7 @@ import {objFilter} from './utils/obj-filter.js'; import {applyExtends} from './utils/apply-extends.js'; import { applyMiddleware, - globalMiddlewareFactory, + GlobalMiddleware, MiddlewareCallback, Middleware, } from './middleware.js'; @@ -75,16 +75,14 @@ function Yargs( let command: CommandInstance; let completion: CompletionInstance | null = null; let groups: Dictionary = {}; - const globalMiddleware: Middleware[] = []; let output = ''; const preservedGroups: Dictionary = {}; + const globalMiddleware = new GlobalMiddleware(self); let usage: UsageInstance; let validation: ValidationInstance; const y18n = shim.y18n; - self.middleware = globalMiddlewareFactory(globalMiddleware, self); - self.scriptName = function (scriptName) { self.customScriptName = true; self.$0 = scriptName; @@ -248,6 +246,7 @@ function Yargs( ? command.reset() : Command(self, usage, validation, globalMiddleware, shim); if (!completion) completion = Completion(self, usage, command, shim); + globalMiddleware.reset(); completionCommand = null; output = ''; @@ -281,6 +280,7 @@ function Yargs( usage.freeze(); validation.freeze(); command.freeze(); + globalMiddleware.freeze(); } function unfreeze() { const frozen = frozens.pop(); @@ -306,6 +306,7 @@ function Yargs( usage.unfreeze(); validation.unfreeze(); command.unfreeze(); + globalMiddleware.unfreeze(); } self.boolean = function (keys) { @@ -847,12 +848,56 @@ function Yargs( return self; }; - self.check = function (f, _global) { - argsert(' [boolean]', [f, _global], arguments.length); - validation.check(f, _global !== false); + self.check = function (f, global) { + argsert(' [boolean]', [f, global], arguments.length); + self.middleware( + ( + argv: Arguments, + yargs: YargsInstance + ): Partial | Promise> => { + let result; + try { + result = f(argv, yargs); + } catch (err) { + console.info('gots here 0', result); + usage.fail(err.message ? err.message : err, err); + return argv; + } + if (isPromise(result)) { + return result.then((result: any) => { + checkResult(result); + return argv; + }); + } else { + checkResult(result); + return argv; + } + function checkResult(result: any) { + if (!result) { + usage.fail(y18n.__('Argument check failed: %s', f.toString())); + } else if (typeof result === 'string' || result instanceof Error) { + usage.fail(result.toString(), result); + } + } + }, + false, + global + ); return self; }; + self.middleware = ( + callback: MiddlewareCallback | MiddlewareCallback[], + applyBeforeValidation?: boolean, + global = true + ) => { + return globalMiddleware.addMiddleware( + callback, + !!applyBeforeValidation, + global + ); + }; + self.global = function global(globals, global) { argsert(' [boolean]', [globals, global], arguments.length); globals = ([] as string[]).concat(globals); @@ -1688,11 +1733,27 @@ function Yargs( // if we're executed via bash completion, don't // bother with validation. if (!requestCompletions) { + // TODO(@bcoe): refactor logic below into new helper: const validation = self._runValidation(aliases, {}, parsed.error); if (!calledFromCommand) { - argvPromise = applyMiddleware(argv, self, globalMiddleware, true); + argvPromise = applyMiddleware( + argv, + self, + globalMiddleware.getMiddleware(), + true + ); } argvPromise = validateAsync(validation, argvPromise ?? argv); + if (isPromise(argvPromise) && !calledFromCommand) { + argvPromise = argvPromise.then(() => { + return applyMiddleware( + argv, + self, + globalMiddleware.getMiddleware(), + false + ); + }); + } } } } catch (err) { @@ -1746,7 +1807,12 @@ function Yargs( argv = self._parsePositionalNumbers(argv); } if (runGlobalMiddleware) { - argv = applyMiddleware(argv, self, globalMiddleware, false); + argv = applyMiddleware( + argv, + self, + globalMiddleware.getMiddleware(), + false + ); } return argv; }; @@ -1813,7 +1879,6 @@ function Yargs( } else if (strictOptions) { validation.unknownArguments(argv, aliases, {}, false, false); } - validation.customChecks(argv, aliases); validation.limitedChoices(argv); validation.implications(argv); validation.conflicting(argv); @@ -1902,8 +1967,8 @@ export interface YargsInstance { array(keys: string | string[]): YargsInstance; boolean(keys: string | string[]): YargsInstance; check( - f: (argv: Arguments, aliases: Dictionary) => any, - _global?: boolean + f: (argv: Arguments, yargs: YargsInstance) => any, + global?: boolean ): YargsInstance; choices: { (keys: string | string[], choices: string | string[]): YargsInstance; @@ -2028,7 +2093,8 @@ export interface YargsInstance { }; middleware( callback: MiddlewareCallback | MiddlewareCallback[], - applyBeforeValidation?: boolean + applyBeforeValidation?: boolean, + global?: boolean ): YargsInstance; nargs: { (keys: string | string[], nargs: number): YargsInstance; diff --git a/package.json b/package.json index ceeaca2d2..60c83ecea 100644 --- a/package.json +++ b/package.json @@ -79,8 +79,8 @@ "fix": "gts fix && npm run fix:js", "fix:js": "eslint . --ext cjs --ext mjs --ext js --fix", "posttest": "npm run check", - "test": "c8 mocha ./test/*.cjs --require ./test/before.cjs --timeout=12000 --check-leaks", - "test:esm": "c8 mocha ./test/esm/*.mjs --check-leaks", + "test": "c8 mocha --enable-source-maps ./test/*.cjs --require ./test/before.cjs --timeout=12000 --check-leaks", + "test:esm": "c8 mocha --enable-source-maps ./test/esm/*.mjs --check-leaks", "coverage": "c8 report --check-coverage", "prepare": "npm run compile", "pretest": "npm run compile -- -p tsconfig.test.json && cross-env NODE_ENV=test npm run build:cjs", diff --git a/test/command.cjs b/test/command.cjs index 5a3c01269..c445fe5d7 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -1250,6 +1250,29 @@ describe('Command', () => { checkCalled.should.equal(false); }); + it('allows each builder to specify own middleware', () => { + let checkCalled1 = 0; + let checkCalled2 = 0; + const y = yargs() + .command('command ', 'a command', () => { + yargs.check(argv => { + checkCalled1++; + return true; + }); + }) + .command('command2 ', 'a second command', yargs => { + yargs.check(argv => { + checkCalled2++; + return true; + }); + }); + y.parse('command blerg --foo'); + y.parse('command2 blerg --foo'); + y.parse('command blerg --foo'); + checkCalled1.should.equal(2); + checkCalled2.should.equal(1); + }); + it('applies demandOption globally', done => { yargs('command blerg --foo') .command('command ', 'a command') diff --git a/test/middleware.cjs b/test/middleware.cjs index ed1074c81..2f4e818ca 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -3,7 +3,7 @@ /* eslint-disable no-unused-vars */ const {expect} = require('chai'); -const {globalMiddlewareFactory} = require('../build/index.cjs'); +const {assert} = require('console'); let yargs; require('chai').should(); @@ -12,6 +12,12 @@ function clearRequireCache() { delete require.cache[require.resolve('../build/index.cjs')]; } +async function wait() { + return Promise(resolve => { + setTimeout(resolve, 10); + }); +} + describe('middleware', () => { beforeEach(() => { yargs = require('../index.cjs'); @@ -20,30 +26,6 @@ describe('middleware', () => { clearRequireCache(); }); - it('should add a list of callbacks to global middleware', () => { - const globalMiddleware = []; - - globalMiddlewareFactory(globalMiddleware)([() => {}, () => {}]); - - globalMiddleware.should.have.lengthOf(2); - }); - - it('should throw exception if middleware is not a function', () => { - const globalMiddleware = []; - - expect(() => { - globalMiddlewareFactory(globalMiddleware)(['callback1', 'callback2']); - }).to.throw('middleware must be a function'); - }); - - it('should add a single callback to global middleware', () => { - const globalMiddleware = []; - - globalMiddlewareFactory(globalMiddleware)(() => {}); - - globalMiddleware.should.have.lengthOf(1); - }); - it('runs the middleware before reaching the handler', done => { yargs(['mw']) .middleware(argv => { @@ -623,4 +605,54 @@ describe('middleware', () => { .parse(); argv.foo.should.equal(198); }); + + it('throws error if middleware not function', () => { + let err; + try { + yargs('snuh --foo 99').middleware(['hello']).parse(); + } catch (_err) { + err = _err; + } + err.message.should.match(/middleware must be a function/); + }); + + describe('async check', () => { + describe('success', () => { + it('returns promise if check is async', async () => { + const argvPromise = yargs('--foo 100') + .middleware(argv => { + argv.foo *= 2; + }, true) + .check(async (argv, yargs) => { + wait(); + if (argv.foo < 200) return false; + else return true; + }) + .parse(); + (!!argvPromise.then).should.equal(true); + const argv = await argvPromise; + argv.foo.should.equal(200); + }); + it('returns promise if check and middleware is async', async () => { + const argvPromise = yargs('--foo 100') + .middleware(async argv => { + wait(); + argv.foo *= 2; + }, true) + .check(async (argv, yargs) => { + wait(); + if (argv.foo < 200) return false; + else return true; + }) + .parse(); + (!!argvPromise.then).should.equal(true); + const argv = await argvPromise; + argv.foo.should.equal(200); + }); + }); + // TODO: add test that demonstrates using yargs instance to get alias. + // TODO: document that check now returns instance rather than aliases. + // TODO: add test that demonstrates using command with middleware and check. + // TODO: add test that demonstrates using command witn check. + }); }); diff --git a/test/usage.cjs b/test/usage.cjs index 355455bbb..41b8aae22 100644 --- a/test/usage.cjs +++ b/test/usage.cjs @@ -4175,6 +4175,7 @@ describe('usage tests', () => { describe('help message caching', () => { it('should display proper usage when an async handler fails', done => { const y = yargs() + .scriptName('mocha') .command('cmd', 'test command', {}, () => { return new Promise((resolve, reject) => setTimeout(reject, 10)); }) @@ -4207,8 +4208,9 @@ describe('usage tests', () => { it('should not display a cached help message for the next parsing', done => { const y = yargs() + .scriptName('mocha') .command('cmd', 'test command', {}, () => { - return new Promise((resolve, reject) => setTimeout(resolve, 10)); + return new Promise((resolve, _reject) => setTimeout(resolve, 10)); }) .demandCommand(1, 'You need at least one command before moving on') .exitProcess(false); From d74999ff532681f6a318ea511dd17e32591fab9a Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 21 Feb 2021 16:41:31 -0800 Subject: [PATCH 2/8] feat: implementing coerce as middleware --- lib/command.ts | 47 +++++++++-------- lib/middleware.ts | 13 +++++ lib/yargs-factory.ts | 119 ++++++++++++++++++++++++++++++------------- test/middleware.cjs | 28 ++++++++++ test/yargs.cjs | 2 +- 5 files changed, 151 insertions(+), 58 deletions(-) diff --git a/lib/command.ts b/lib/command.ts index 116c5c617..1969101ad 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -24,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)/; @@ -314,16 +315,16 @@ 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( + innerArgv, + (err: Error) => { + throw err; + }, + result => { + validation(result); + return result; + } + ); } if (commandHandler.handler && !yargs._hasOutput()) { @@ -336,18 +337,20 @@ 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); - if (isPromise(handlerResult)) { - const innerArgvRef = innerArgv; - innerArgv = handlerResult.then(() => innerArgvRef); + innerArgv = maybeAsyncResult( + innerArgv, + (err: Error) => { + throw err; + }, + result => { + const handlerResult = commandHandler.handler(result as Arguments); + if (isPromise(handlerResult)) { + return handlerResult.then(() => result); + } else { + return result; + } } - } + ); yargs.getUsageInstance().cacheHelpMessage(); if (isPromise(innerArgv) && !yargs._hasParseCallback()) { @@ -493,7 +496,7 @@ export function command( if (!unparsed.length) return; const config: Configuration = Object.assign({}, options.configuration, { - 'populate--': true, + 'populate--': false, }); const parsed = shim.Parser.detailed( unparsed, diff --git a/lib/middleware.ts b/lib/middleware.ts index 70594185d..0b9670827 100644 --- a/lib/middleware.ts +++ b/lib/middleware.ts @@ -40,6 +40,18 @@ export class GlobalMiddleware { } return this.yargs; } + // For "coerce" middleware, only one middleware instance can be registered + // per option: + addCoerceMiddleware( + callback: MiddlewareCallback | MiddlewareCallback[], + option: string + ): YargsInstance { + const aliases = this.yargs.getAliases(); + this.globalMiddleware = this.globalMiddleware.filter(m => { + return true; + }); + return this.addMiddleware(callback, true, true); + } getMiddleware() { return this.globalMiddleware; } @@ -108,4 +120,5 @@ export interface MiddlewareCallback { export interface Middleware extends MiddlewareCallback { applyBeforeValidation: boolean; global: boolean; + option?: string; } diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index cda8c323f..3abe3f285 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -58,6 +58,7 @@ import { Middleware, } from './middleware.js'; import {isPromise} from './utils/is-promise.js'; +import {maybeAsyncResult} from './utils/maybe-async-result.js'; import setBlocking from './utils/set-blocking.js'; let shim: PlatformShim; @@ -221,7 +222,6 @@ function Yargs( 'choices', 'demandedOptions', 'demandedCommands', - 'coerce', 'deprecatedOptions', ]; @@ -492,7 +492,57 @@ function Yargs( [keys, value], arguments.length ); - populateParserHintSingleValueDictionary(self.coerce, 'coerce', keys, value); + if (Array.isArray(keys)) { + if (!value) { + throw new YError('coerce callback must be provided'); + } + for (const key of keys) { + self.coerce(key, value!); + } + return self; + } else if (typeof keys === 'object') { + for (const key of Object.keys(keys)) { + self.coerce(key, keys[key]); + } + return self; + } + if (!value) { + throw new YError('coerce callback must be provided'); + } + // This noop tells yargs-parser about the existence of the option + // represented by "keys", so that it can apply camel case expansion + // if needed: + self.alias(keys, keys); + self.middleware( + ( + argv: Arguments, + yargs: YargsInstance + ): Partial | Promise> => { + let aliases: Dictionary; + return maybeAsyncResult< + Partial | Promise> | any + >( + () => { + aliases = yargs.getAliases(); + return value(argv[keys]); + }, + (err: Error): Partial | Promise> => { + throw new YError(err.message); + }, + (result: any): Partial => { + argv[keys] = result; + if (aliases[keys]) { + for (const alias of aliases[keys]) { + argv[alias] = result; + } + } + return argv; + } + ); + }, + true, + true + ); return self; }; @@ -759,6 +809,10 @@ function Yargs( return self; }; + self.getAliases = () => { + return self.parsed ? self.parsed.aliases : {}; + }; + self.getDemandedOptions = () => { argsert([], 0); return options.demandedOptions; @@ -855,30 +909,25 @@ function Yargs( argv: Arguments, yargs: YargsInstance ): Partial | Promise> => { - let result; - try { - result = f(argv, yargs); - } catch (err) { - console.info('gots here 0', result); - usage.fail(err.message ? err.message : err, err); - return argv; - } - if (isPromise(result)) { - return result.then((result: any) => { - checkResult(result); + return maybeAsyncResult< + Partial | Promise> | any + >( + () => { + return f(argv, yargs); + }, + (err: Error): Partial | Promise> => { + usage.fail(err.message ? err.message : err.toString(), err); + return argv; + }, + (result: any): Partial | Promise> => { + if (!result) { + usage.fail(y18n.__('Argument check failed: %s', f.toString())); + } else if (typeof result === 'string' || result instanceof Error) { + usage.fail(result.toString(), result); + } return argv; - }); - } else { - checkResult(result); - return argv; - } - function checkResult(result: any) { - if (!result) { - usage.fail(y18n.__('Argument check failed: %s', f.toString())); - } else if (typeof result === 'string' || result instanceof Error) { - usage.fail(result.toString(), result); } - } + ); }, false, global @@ -1774,17 +1823,16 @@ function Yargs( validation: (argv: Arguments) => void, argv: Arguments | Promise ): Arguments | Promise { - if (isPromise(argv)) { - // If the middlware returned a promise, resolve the middleware - // before applying the validation: - argv = argv.then(argv => { - validation(argv); - return argv; - }); - } else { - validation(argv); - } - return argv; + return maybeAsyncResult( + argv, + (err: Error) => { + throw err; + }, + result => { + validation(result); + return result; + } + ); } // Applies a couple post processing steps that are easier to perform @@ -2066,6 +2114,7 @@ export interface YargsInstance { ): Promise | any; getHelp(): Promise; getContext(): Context; + getAliases(): Dictionary; getDemandedCommands(): Options['demandedCommands']; getDemandedOptions(): Options['demandedOptions']; getDeprecatedOptions(): Options['deprecatedOptions']; diff --git a/test/middleware.cjs b/test/middleware.cjs index 2f4e818ca..ad59d9451 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -4,6 +4,7 @@ const {expect} = require('chai'); const {assert} = require('console'); +const {rawListeners} = require('process'); let yargs; require('chai').should(); @@ -649,10 +650,37 @@ describe('middleware', () => { const argv = await argvPromise; argv.foo.should.equal(200); }); + it('allows two commands to register different coerce methods', async () => { + const y = yargs() + .command('command1', 'first command', yargs => { + yargs.coerce('foo', async arg => { + wait(); + return new Date(arg); + }); + }) + .command('command2', 'second command', yargs => { + yargs.coerce('foo', async arg => { + wait(); + return new Number(arg); + }); + }) + .coerce('foo', async _arg => { + return 'hello'; + }); + const r1 = await y.parse('command1 --foo 2020-10-10'); + expect(r1.foo).to.be.an.instanceof(Date); + const r2 = await y.parse('command2 --foo 302'); + r2.foo.should.equal(302); + }); }); // TODO: add test that demonstrates using yargs instance to get alias. // TODO: document that check now returns instance rather than aliases. // TODO: add test that demonstrates using command with middleware and check. // TODO: add test that demonstrates using command witn check. + + // TODO: add test for two commands that register the same coerce. + // TODO: add test for async coerce happening prior to validation. }); + + // TODO: add test for coerce happening prior to validation. }); diff --git a/test/yargs.cjs b/test/yargs.cjs index e279ec425..fa7753687 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -303,7 +303,6 @@ describe('yargs dsl tests', () => { narg: {}, defaultDescription: {}, choices: {}, - coerce: {}, skipValidation: [], count: [], normalize: [], @@ -1985,6 +1984,7 @@ describe('yargs dsl tests', () => { '--file', path.join(__dirname, 'fixtures', 'package.json'), ]) + .alias('file', 'f') .coerce('file', arg => JSON.parse(fs.readFileSync(arg, 'utf8'))) .parse(); expect(argv.file).to.have.property('version').and.equal('9.9.9'); From 3a0169e5c4c82aa28c75949d709b42ca7904f203 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 21 Feb 2021 16:41:37 -0800 Subject: [PATCH 3/8] feat: implementing coerce as middleware --- lib/utils/maybe-async-result.ts | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 lib/utils/maybe-async-result.ts diff --git a/lib/utils/maybe-async-result.ts b/lib/utils/maybe-async-result.ts new file mode 100644 index 000000000..2a419fff1 --- /dev/null +++ b/lib/utils/maybe-async-result.ts @@ -0,0 +1,23 @@ +import {isPromise} from './is-promise.js'; +export function maybeAsyncResult( + getResult: (() => T | Promise) | T | Promise, + errorHandler: (err: Error) => T, + resultHandler: (result: T) => T | Promise +): T | Promise { + 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'; +} From 0862d7f087e3fa761da44ce94b157ce119546e54 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 21 Feb 2021 18:10:40 -0800 Subject: [PATCH 4/8] feat: continue work to implement coerce --- lib/middleware.ts | 7 +++-- lib/utils/maybe-async-result.ts | 7 +++++ lib/yargs-factory.ts | 5 ++- test/middleware.cjs | 54 ++++++++++++++++----------------- 4 files changed, 41 insertions(+), 32 deletions(-) diff --git a/lib/middleware.ts b/lib/middleware.ts index 0b9670827..35d35463d 100644 --- a/lib/middleware.ts +++ b/lib/middleware.ts @@ -43,13 +43,16 @@ export class GlobalMiddleware { // For "coerce" middleware, only one middleware instance can be registered // per option: addCoerceMiddleware( - callback: MiddlewareCallback | MiddlewareCallback[], + callback: MiddlewareCallback, option: string ): YargsInstance { const aliases = this.yargs.getAliases(); this.globalMiddleware = this.globalMiddleware.filter(m => { - return true; + 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() { diff --git a/lib/utils/maybe-async-result.ts b/lib/utils/maybe-async-result.ts index 2a419fff1..50c2e8e7d 100644 --- a/lib/utils/maybe-async-result.ts +++ b/lib/utils/maybe-async-result.ts @@ -1,3 +1,10 @@ +// 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( getResult: (() => T | Promise) | T | Promise, diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 3abe3f285..a82e10002 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -513,7 +513,7 @@ function Yargs( // represented by "keys", so that it can apply camel case expansion // if needed: self.alias(keys, keys); - self.middleware( + globalMiddleware.addCoerceMiddleware( ( argv: Arguments, yargs: YargsInstance @@ -540,8 +540,7 @@ function Yargs( } ); }, - true, - true + keys ); return self; }; diff --git a/test/middleware.cjs b/test/middleware.cjs index ad59d9451..0b1c76869 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -3,8 +3,6 @@ /* eslint-disable no-unused-vars */ const {expect} = require('chai'); -const {assert} = require('console'); -const {rawListeners} = require('process'); let yargs; require('chai').should(); @@ -650,37 +648,39 @@ describe('middleware', () => { const argv = await argvPromise; argv.foo.should.equal(200); }); - it('allows two commands to register different coerce methods', async () => { - const y = yargs() - .command('command1', 'first command', yargs => { - yargs.coerce('foo', async arg => { - wait(); - return new Date(arg); - }); - }) - .command('command2', 'second command', yargs => { - yargs.coerce('foo', async arg => { - wait(); - return new Number(arg); - }); - }) - .coerce('foo', async _arg => { - return 'hello'; - }); - const r1 = await y.parse('command1 --foo 2020-10-10'); - expect(r1.foo).to.be.an.instanceof(Date); - const r2 = await y.parse('command2 --foo 302'); - r2.foo.should.equal(302); - }); }); // TODO: add test that demonstrates using yargs instance to get alias. // TODO: document that check now returns instance rather than aliases. // TODO: add test that demonstrates using command with middleware and check. - // TODO: add test that demonstrates using command witn check. + // TODO: add test that demonstrates using command with check. + }); + describe('async coerce', () => { + it('allows two commands to register different coerce methods', async () => { + const y = yargs() + .command('command1', 'first command', yargs => { + yargs.coerce('foo', async arg => { + wait(); + return new Date(arg); + }); + }) + .command('command2', 'second command', yargs => { + yargs.coerce('foo', async arg => { + wait(); + return new Number(arg); + }); + }) + .coerce('foo', async _arg => { + return 'hello'; + }); + const r1 = await y.parse('command1 --foo 2020-10-10'); + expect(r1.foo).to.be.an.instanceof(Date); + const r2 = await y.parse('command2 --foo 302'); + r2.foo.should.equal(302); + }); + // TODO: add test which demonstrates alias used with coerce. // TODO: add test for two commands that register the same coerce. // TODO: add test for async coerce happening prior to validation. + // TODO: add test for coerce happening prior to validation. }); - - // TODO: add test for coerce happening prior to validation. }); From 40c90c552f56954c1ec52ad87980fd7981dc1c99 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 28 Feb 2021 13:02:30 -0800 Subject: [PATCH 5/8] test: adds additional tests for check --- docs/api.md | 2 +- lib/yargs-factory.ts | 9 +-- test/middleware.cjs | 163 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 161 insertions(+), 13 deletions(-) diff --git a/docs/api.md b/docs/api.md index 1a9704c5c..14a115f95 100644 --- a/docs/api.md +++ b/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 diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index f05af1681..93beb60f1 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -906,13 +906,13 @@ function Yargs( self.middleware( ( argv: Arguments, - yargs: YargsInstance + _yargs: YargsInstance ): Partial | Promise> => { return maybeAsyncResult< Partial | Promise> | any >( () => { - return f(argv, yargs); + return f(argv); }, (err: Error): Partial | Promise> => { usage.fail(err.message ? err.message : err.toString(), err); @@ -2013,10 +2013,7 @@ export interface YargsInstance { }; array(keys: string | string[]): YargsInstance; boolean(keys: string | string[]): YargsInstance; - check( - f: (argv: Arguments, yargs: YargsInstance) => any, - global?: boolean - ): YargsInstance; + check(f: (argv: Arguments) => any, global?: boolean): YargsInstance; choices: { (keys: string | string[], choices: string | string[]): YargsInstance; (keyChoices: Dictionary): YargsInstance; diff --git a/test/middleware.cjs b/test/middleware.cjs index 0b1c76869..764b9bde6 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -622,7 +622,7 @@ describe('middleware', () => { .middleware(argv => { argv.foo *= 2; }, true) - .check(async (argv, yargs) => { + .check(async argv => { wait(); if (argv.foo < 200) return false; else return true; @@ -638,7 +638,7 @@ describe('middleware', () => { wait(); argv.foo *= 2; }, true) - .check(async (argv, yargs) => { + .check(async argv => { wait(); if (argv.foo < 200) return false; else return true; @@ -648,11 +648,162 @@ describe('middleware', () => { const argv = await argvPromise; argv.foo.should.equal(200); }); + it('allows async check to be used with command', async () => { + let output = ''; + const argv = await yargs('cmd --foo 300') + .command( + 'cmd', + 'a command', + yargs => { + yargs.check(async argv => { + wait(); + output += 'first'; + if (argv.foo < 200) return false; + else return true; + }); + }, + async argv => { + wait(); + output += 'second'; + } + ) + .parse(); + argv._.should.include('cmd'); + argv.foo.should.equal(300); + output.should.equal('firstsecond'); + }); + it('allows async check to be used with command and middleware', async () => { + let output = ''; + const argv = await yargs('cmd --foo 100') + .command( + 'cmd', + 'a command', + yargs => { + yargs.check(async argv => { + wait(); + output += 'second'; + if (argv.foo < 200) return false; + else return true; + }); + }, + async argv => { + wait(); + output += 'fourth'; + }, + [ + async argv => { + wait(); + output += 'third'; + argv.foo *= 2; + }, + ] + ) + .middleware(async argv => { + wait(); + output += 'first'; + argv.foo *= 2; + }, true) + .parse(); + argv._.should.include('cmd'); + argv.foo.should.equal(400); + output.should.equal('firstsecondthirdfourth'); + }); + }); + describe('failure', () => { + it('allows failed check to be caught', async () => { + try { + await yargs('--f 33') + .alias('foo', 'f') + .fail(false) + .check(async argv => { + wait(); + return argv.foo > 50; + }) + .parse(); + throw Error('unreachable'); + } catch (err) { + err.message.should.match(/Argument check failed/); + } + }); + it('allows error to be caught before calling command', async () => { + let output = ''; + try { + await yargs('cmd --foo 100') + .fail(false) + .command( + 'cmd', + 'a command', + yargs => { + yargs.check(async argv => { + wait(); + output += 'first'; + if (argv.foo < 200) return false; + else return true; + }); + }, + async argv => { + wait(); + output += 'second'; + } + ) + .parse(); + throw Error('unreachable'); + } catch (err) { + output.should.equal('first'); + err.message.should.match(/Argument check failed/); + } + }); + it('allows error to be caught before calling command and middleware', async () => { + let output = ''; + try { + const argv = await yargs('cmd --foo 10') + .fail(false) + .command( + 'cmd', + 'a command', + yargs => { + yargs.check(async argv => { + wait(); + output += 'second'; + if (argv.foo < 200) return false; + else return true; + }); + }, + async argv => { + wait(); + output += 'fourth'; + }, + [ + async argv => { + wait(); + output += 'third'; + argv.foo *= 2; + }, + ] + ) + .middleware(async argv => { + wait(); + output += 'first'; + argv.foo *= 2; + }, true) + .parse(); + throw Error('unreachable'); + } catch (err) { + err.message.should.match(/Argument check failed/); + output.should.equal('firstsecond'); + } + }); + }); + it('applies alliases prior to calling check', async () => { + const argv = await yargs('--f 99') + .alias('foo', 'f') + .check(async argv => { + wait(); + return argv.foo > 50; + }) + .parse(); + argv.foo.should.equal(99); }); - // TODO: add test that demonstrates using yargs instance to get alias. - // TODO: document that check now returns instance rather than aliases. - // TODO: add test that demonstrates using command with middleware and check. - // TODO: add test that demonstrates using command with check. }); describe('async coerce', () => { From 04fc58d945524ce7f5fed57cbc201008d6d81aae Mon Sep 17 00:00:00 2001 From: bcoe Date: Sun, 28 Feb 2021 18:55:10 -0800 Subject: [PATCH 6/8] test: flesh out missing tests --- lib/command.ts | 34 ++++++++--------------- lib/utils/maybe-async-result.ts | 6 ++-- lib/yargs-factory.ts | 29 ++++++++----------- test/command.cjs | 19 +++++++++++++ test/middleware.cjs | 49 ++++++++++++++++++++++++++++++--- 5 files changed, 90 insertions(+), 47 deletions(-) diff --git a/lib/command.ts b/lib/command.ts index 1969101ad..cb02ebf6f 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -315,16 +315,10 @@ export function command( (yargs.parsed as DetailedArguments).error, !command ); - innerArgv = maybeAsyncResult( - innerArgv, - (err: Error) => { - throw err; - }, - result => { - validation(result); - return result; - } - ); + innerArgv = maybeAsyncResult(innerArgv, result => { + validation(result); + return result; + }); } if (commandHandler.handler && !yargs._hasOutput()) { @@ -337,20 +331,14 @@ export function command( yargs._postProcess(innerArgv, populateDoubleDash, false, false); innerArgv = applyMiddleware(innerArgv, yargs, middlewares, false); - innerArgv = maybeAsyncResult( - innerArgv, - (err: Error) => { - throw err; - }, - result => { - const handlerResult = commandHandler.handler(result as Arguments); - if (isPromise(handlerResult)) { - return handlerResult.then(() => result); - } else { - return result; - } + innerArgv = maybeAsyncResult(innerArgv, result => { + const handlerResult = commandHandler.handler(result as Arguments); + if (isPromise(handlerResult)) { + return handlerResult.then(() => result); + } else { + return result; } - ); + }); yargs.getUsageInstance().cacheHelpMessage(); if (isPromise(innerArgv) && !yargs._hasParseCallback()) { diff --git a/lib/utils/maybe-async-result.ts b/lib/utils/maybe-async-result.ts index 50c2e8e7d..1e288cb11 100644 --- a/lib/utils/maybe-async-result.ts +++ b/lib/utils/maybe-async-result.ts @@ -8,8 +8,10 @@ import {isPromise} from './is-promise.js'; export function maybeAsyncResult( getResult: (() => T | Promise) | T | Promise, - errorHandler: (err: Error) => T, - resultHandler: (result: T) => T | Promise + resultHandler: (result: T) => T | Promise, + errorHandler: (err: Error) => T = (err: Error) => { + throw err; + } ): T | Promise { try { const result = isFunction(getResult) ? getResult() : getResult; diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 93beb60f1..7fb2ebcd2 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -526,9 +526,6 @@ function Yargs( aliases = yargs.getAliases(); return value(argv[keys]); }, - (err: Error): Partial | Promise> => { - throw new YError(err.message); - }, (result: any): Partial => { argv[keys] = result; if (aliases[keys]) { @@ -537,6 +534,9 @@ function Yargs( } } return argv; + }, + (err: Error): Partial | Promise> => { + throw new YError(err.message); } ); }, @@ -914,10 +914,6 @@ function Yargs( () => { return f(argv); }, - (err: Error): Partial | Promise> => { - usage.fail(err.message ? err.message : err.toString(), err); - return argv; - }, (result: any): Partial | Promise> => { if (!result) { usage.fail(y18n.__('Argument check failed: %s', f.toString())); @@ -925,6 +921,10 @@ function Yargs( usage.fail(result.toString(), result); } return argv; + }, + (err: Error): Partial | Promise> => { + usage.fail(err.message ? err.message : err.toString(), err); + return argv; } ); }, @@ -1781,7 +1781,6 @@ function Yargs( // if we're executed via bash completion, don't // bother with validation. if (!requestCompletions) { - // TODO(@bcoe): refactor logic below into new helper: const validation = self._runValidation(aliases, {}, parsed.error); if (!calledFromCommand) { argvPromise = applyMiddleware( @@ -1822,16 +1821,10 @@ function Yargs( validation: (argv: Arguments) => void, argv: Arguments | Promise ): Arguments | Promise { - return maybeAsyncResult( - argv, - (err: Error) => { - throw err; - }, - result => { - validation(result); - return result; - } - ); + return maybeAsyncResult(argv, result => { + validation(result); + return result; + }); } // Applies a couple post processing steps that are easier to perform diff --git a/test/command.cjs b/test/command.cjs index c445fe5d7..576b7707c 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -1908,4 +1908,23 @@ describe('Command', () => { yargs().command(cmds).parse('a c 10 5', context); context.output.value.should.equal(15); }); + + it('allows async exception in handler to be caught', async () => { + try { + const argv = await yargs(['mw']) + .fail(false) + .command( + 'mw', + 'adds func to middleware', + () => {}, + async () => { + throw Error('not cool'); + } + ) + .parse(); + throw Error('unreachable'); + } catch (err) { + err.message.should.match(/not cool/); + } + }); }); diff --git a/test/middleware.cjs b/test/middleware.cjs index 764b9bde6..61159c2ce 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -829,9 +829,50 @@ describe('middleware', () => { const r2 = await y.parse('command2 --foo 302'); r2.foo.should.equal(302); }); - // TODO: add test which demonstrates alias used with coerce. - // TODO: add test for two commands that register the same coerce. - // TODO: add test for async coerce happening prior to validation. - // TODO: add test for coerce happening prior to validation. + it('coerce is applied to argument and aliases', async () => { + let callCount = 0; + const argvPromise = yargs() + .alias('f', 'foo') + .coerce('foo', async arg => { + wait(); + callCount++; + return new Date(arg.toString()); + }) + .parse('-f 2014'); + (!!argvPromise.then).should.equal(true); + const argv = await argvPromise; + callCount.should.equal(1); + expect(argv.foo).to.be.an.instanceof(Date); + expect(argv.f).to.be.an.instanceof(Date); + }); + it('applies coerce before validation', async () => { + const argv = await yargs() + .option('foo', { + choices: [10, 20, 30], + }) + .coerce('foo', async arg => { + wait(); + return (arg *= 2); + }) + .parse('--foo 5'); + argv.foo.should.equal(10); + }); + it('allows error to be caught', async () => { + try { + await yargs() + .fail(false) + .option('foo', { + choices: [10, 20, 30], + }) + .coerce('foo', async arg => { + wait(); + return (arg *= 2); + }) + .parse('--foo 2'); + throw Error('unreachable'); + } catch (err) { + err.message.should.match(/Choices: 10, 20, 30/); + } + }); }); }); From 581eb30eb937178b42da6011e17b20950f84be7a Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 13 Mar 2021 18:36:36 -0800 Subject: [PATCH 7/8] test: get coverage back to 100% --- lib/command.ts | 1 + lib/yargs-factory.ts | 2 +- test/command.cjs | 21 +++++++++++++--- test/middleware.cjs | 60 ++++++++++++++++++-------------------------- test/yargs.cjs | 10 ++++++++ 5 files changed, 55 insertions(+), 39 deletions(-) diff --git a/lib/command.ts b/lib/command.ts index 161798ef7..5434c1149 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -494,6 +494,7 @@ export function command( const config: Configuration = Object.assign({}, options.configuration, { 'populate--': false, }); + const parsed = shim.Parser.detailed( unparsed, Object.assign({}, options, { diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index ad52ca5b0..7a9185928 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -497,7 +497,7 @@ function Yargs( throw new YError('coerce callback must be provided'); } for (const key of keys) { - self.coerce(key, value!); + self.coerce(key, value); } return self; } else if (typeof keys === 'object') { diff --git a/test/command.cjs b/test/command.cjs index 576b7707c..59f05ada4 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -1,6 +1,7 @@ /* global describe, it, beforeEach */ /* eslint-disable no-unused-vars */ 'use strict'; +const assert = require('assert'); const yargs = require('../index.cjs'); const expect = require('chai').expect; const checkOutput = require('./helpers/utils.cjs').checkOutput; @@ -1910,8 +1911,8 @@ describe('Command', () => { }); it('allows async exception in handler to be caught', async () => { - try { - const argv = await yargs(['mw']) + await assert.rejects( + yargs(['mw']) .fail(false) .command( 'mw', @@ -1921,10 +1922,24 @@ describe('Command', () => { throw Error('not cool'); } ) + .parse(), + /not cool/ + ); + }); + + it('reports error if error occurs parsing positional', () => { + try { + yargs(['cmd', 'neat']) + .fail(false) + .command('cmd ', 'run a foo command', yargs => { + yargs.option('foo', { + nargs: 3, + }); + }) .parse(); throw Error('unreachable'); } catch (err) { - err.message.should.match(/not cool/); + err.message.should.match(/Not enough arguments/); } }); }); diff --git a/test/middleware.cjs b/test/middleware.cjs index 61159c2ce..625be7d3d 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -2,6 +2,7 @@ /* global describe, it, beforeEach, afterEach */ /* eslint-disable no-unused-vars */ +const assert = require('assert'); const {expect} = require('chai'); let yargs; require('chai').should(); @@ -607,12 +608,9 @@ describe('middleware', () => { it('throws error if middleware not function', () => { let err; - try { + assert.throws(() => { yargs('snuh --foo 99').middleware(['hello']).parse(); - } catch (_err) { - err = _err; - } - err.message.should.match(/middleware must be a function/); + }, /middleware must be a function/); }); describe('async check', () => { @@ -711,24 +709,22 @@ describe('middleware', () => { }); describe('failure', () => { it('allows failed check to be caught', async () => { - try { - await yargs('--f 33') + await assert.rejects( + yargs('--f 33') .alias('foo', 'f') .fail(false) .check(async argv => { wait(); return argv.foo > 50; }) - .parse(); - throw Error('unreachable'); - } catch (err) { - err.message.should.match(/Argument check failed/); - } + .parse(), + /Argument check failed/ + ); }); it('allows error to be caught before calling command', async () => { let output = ''; - try { - await yargs('cmd --foo 100') + await assert.rejects( + yargs('cmd --foo 100') .fail(false) .command( 'cmd', @@ -746,17 +742,15 @@ describe('middleware', () => { output += 'second'; } ) - .parse(); - throw Error('unreachable'); - } catch (err) { - output.should.equal('first'); - err.message.should.match(/Argument check failed/); - } + .parse(), + /Argument check failed/ + ); + output.should.equal('first'); }); it('allows error to be caught before calling command and middleware', async () => { let output = ''; - try { - const argv = await yargs('cmd --foo 10') + await assert.rejects( + yargs('cmd --foo 10') .fail(false) .command( 'cmd', @@ -786,12 +780,10 @@ describe('middleware', () => { output += 'first'; argv.foo *= 2; }, true) - .parse(); - throw Error('unreachable'); - } catch (err) { - err.message.should.match(/Argument check failed/); - output.should.equal('firstsecond'); - } + .parse(), + /Argument check failed/ + ); + output.should.equal('firstsecond'); }); }); it('applies alliases prior to calling check', async () => { @@ -858,8 +850,8 @@ describe('middleware', () => { argv.foo.should.equal(10); }); it('allows error to be caught', async () => { - try { - await yargs() + await assert.rejects( + yargs() .fail(false) .option('foo', { choices: [10, 20, 30], @@ -868,11 +860,9 @@ describe('middleware', () => { wait(); return (arg *= 2); }) - .parse('--foo 2'); - throw Error('unreachable'); - } catch (err) { - err.message.should.match(/Choices: 10, 20, 30/); - } + .parse('--foo 2'), + /Choices: 10, 20, 30/ + ); }); }); }); diff --git a/test/yargs.cjs b/test/yargs.cjs index fa7753687..d8e016280 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -2,6 +2,7 @@ /* global context, describe, it, beforeEach, afterEach */ /* eslint-disable no-unused-vars */ +const assert = require('assert'); const expect = require('chai').expect; const fs = require('fs'); const path = require('path'); @@ -2138,6 +2139,15 @@ describe('yargs dsl tests', () => { expect(msg).to.equal('ball'); expect(err).to.not.equal(undefined); }); + + it('throws error if coerce callback is missing', () => { + assert.throws(() => { + yargs().coerce(['a', 'b']); + }, /coerce callback must be provided/); + assert.throws(() => { + yargs().coerce('c'); + }, /coerce callback must be provided/); + }); }); describe('stop parsing', () => { From 552f15cd3e48b1bdb08ff0d563f69b1fc8cf0365 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 13 Mar 2021 18:39:34 -0800 Subject: [PATCH 8/8] docs: slight tweaks to docs --- docs/api.md | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/docs/api.md b/docs/api.md index 283e381a7..2440dd318 100644 --- a/docs/api.md +++ b/docs/api.md @@ -175,7 +175,7 @@ var argv = require('yargs/yargs')(process.argv.slice(2)) .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