diff --git a/lib/command.ts b/lib/command.ts index 3509c3574..81a7b50e5 100644 --- a/lib/command.ts +++ b/lib/command.ts @@ -301,18 +301,27 @@ export function command( const middlewares = globalMiddleware .slice(0) .concat(commandHandler.middlewares); - applyMiddleware(innerArgv, yargs, middlewares, true); + innerArgv = applyMiddleware(innerArgv, yargs, middlewares, true); // we apply validation post-hoc, so that custom // checks get passed populated positional arguments. if (!yargs._hasOutput()) { - yargs._runValidation( - innerArgv as Arguments, + const validation = yargs._runValidation( aliases, positionalMap, (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); + } } if (commandHandler.handler && !yargs._hasOutput()) { @@ -322,7 +331,7 @@ export function command( const populateDoubleDash = !!yargs.getOptions().configuration[ 'populate--' ]; - yargs._postProcess(innerArgv, populateDoubleDash); + yargs._postProcess(innerArgv, populateDoubleDash, false, false); innerArgv = applyMiddleware(innerArgv, yargs, middlewares, false); if (isPromise(innerArgv)) { diff --git a/lib/middleware.ts b/lib/middleware.ts index 9f930313a..7028d055b 100644 --- a/lib/middleware.ts +++ b/lib/middleware.ts @@ -49,9 +49,6 @@ export function applyMiddleware( middlewares: Middleware[], beforeValidation: boolean ) { - const beforeValidationError = new Error( - 'middleware cannot return a promise when applyBeforeValidation is true' - ); return middlewares.reduce>( (acc, middleware) => { if (middleware.applyBeforeValidation !== beforeValidation) { @@ -71,8 +68,6 @@ export function applyMiddleware( ); } else { const result = middleware(acc, yargs); - if (beforeValidation && isPromise(result)) throw beforeValidationError; - return isPromise(result) ? result.then(middlewareObj => Object.assign(acc, middlewareObj)) : Object.assign(acc, result); diff --git a/lib/usage.ts b/lib/usage.ts index 308e3b213..d436f209f 100644 --- a/lib/usage.ts +++ b/lib/usage.ts @@ -48,7 +48,8 @@ export function usage(yargs: YargsInstance, y18n: Y18N, shim: PlatformShim) { for (let i = fails.length - 1; i >= 0; --i) { const fail = fails[i]; if (isBoolean(fail)) { - throw err; + if (err) throw err; + else if (msg) throw Error(msg); } else { fail(msg, err, self); } diff --git a/lib/validation.ts b/lib/validation.ts index d259a407f..371374d46 100644 --- a/lib/validation.ts +++ b/lib/validation.ts @@ -101,8 +101,10 @@ export function validation( }; // make sure all the required arguments are present. - self.requiredArguments = function requiredArguments(argv) { - const demandedOptions = yargs.getDemandedOptions(); + self.requiredArguments = function requiredArguments( + argv, + demandedOptions: Dictionary + ) { let missing: Dictionary | null = null; for (const key of Object.keys(demandedOptions)) { @@ -125,7 +127,6 @@ export function validation( } const customMsg = customMsgs.length ? `\n${customMsgs.join('\n')}` : ''; - usage.fail( __n( 'Missing required argument: %s', @@ -486,7 +487,10 @@ export interface ValidationInstance { nonOptionCount(argv: Arguments): void; positionalCount(required: number, observed: number): void; recommendCommands(cmd: string, potentialCommands: string[]): void; - requiredArguments(argv: Arguments): void; + requiredArguments( + argv: Arguments, + demandedOptions: Dictionary + ): void; reset(localLookup: Dictionary): ValidationInstance; unfreeze(): void; unknownArguments( diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index 60335e058..69e79d663 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -52,6 +52,7 @@ import { import {objFilter} from './utils/obj-filter.js'; import {applyExtends} from './utils/apply-extends.js'; import { + applyMiddleware, globalMiddlewareFactory, MiddlewareCallback, Middleware, @@ -1478,11 +1479,11 @@ function Yargs( self._parseArgs = function parseArgs( args: string | string[] | null, shortCircuit?: boolean | null, - _calledFromCommand?: boolean, + calledFromCommand?: boolean, commandIndex = 0, helpOnly = false ) { - let skipValidation = !!_calledFromCommand; + let skipValidation = !!calledFromCommand; args = args || processArgs; options.__ = y18n.__; @@ -1499,7 +1500,9 @@ function Yargs( }) ) as DetailedArguments; - let argv = parsed.argv as Arguments; + let argv: Arguments = parsed.argv as Arguments; + let argvPromise: Arguments | Promise | undefined = undefined; + // Used rather than argv if middleware introduces an async step: if (parseContext) argv = Object.assign({}, argv, parseContext); const aliases = parsed.aliases; @@ -1510,7 +1513,7 @@ function Yargs( // const y = yargs(); y.parse('foo --bar'); yargs.parse('bar --foo'). // When a prior parse has completed and a new parse is beginning, we // need to clear the cached help message from the previous parse: - if (!_calledFromCommand) { + if (commandIndex === 0) { usage.clearCachedHelpMessage(); } @@ -1521,7 +1524,12 @@ function Yargs( // are two passes through the parser. If completion // is being performed short-circuit on the first pass. if (shortCircuit) { - return self._postProcess(argv, populateDoubleDash, _calledFromCommand); + return self._postProcess( + argv, + populateDoubleDash, + !!calledFromCommand, + false // Don't run middleware when figuring out completion. + ); } // if there's a handler associated with a @@ -1563,7 +1571,12 @@ function Yargs( i + 1, helpOnly // Don't run a handler, just figure out the help string. ); - return self._postProcess(innerArgv, populateDoubleDash); + return self._postProcess( + innerArgv, + populateDoubleDash, + !!calledFromCommand, + false + ); } else if (!firstUnknownCommand && cmd !== completionCommand) { firstUnknownCommand = cmd; break; @@ -1579,7 +1592,12 @@ function Yargs( 0, helpOnly ); - return self._postProcess(innerArgv, populateDoubleDash); + return self._postProcess( + innerArgv, + populateDoubleDash, + !!calledFromCommand, + false + ); } // recommend a command if recommendCommands() has @@ -1601,7 +1619,12 @@ function Yargs( } } else if (command.hasDefaultCommand() && !skipDefaultCommand) { const innerArgv = command.runCommand(null, self, parsed, 0, helpOnly); - return self._postProcess(innerArgv, populateDoubleDash); + return self._postProcess( + innerArgv, + populateDoubleDash, + !!calledFromCommand, + false + ); } // we must run completions first, a user might @@ -1622,7 +1645,12 @@ function Yargs( }); self.exit(0); }); - return self._postProcess(argv, !populateDoubleDash, _calledFromCommand); + return self._postProcess( + argv, + !populateDoubleDash, + !!calledFromCommand, + false // Don't run middleware when figuring out completion. + ); } // Handle 'help' and 'version' options @@ -1660,26 +1688,54 @@ function Yargs( // if we're executed via bash completion, don't // bother with validation. if (!requestCompletions) { - self._runValidation(argv, aliases, {}, parsed.error); + const validation = self._runValidation(aliases, {}, parsed.error); + if (!calledFromCommand) { + argvPromise = applyMiddleware(argv, self, globalMiddleware, true); + } + argvPromise = validateAsync(validation, argvPromise ?? argv); } } } catch (err) { if (err instanceof YError) usage.fail(err.message, err); else throw err; } - - return self._postProcess(argv, populateDoubleDash, _calledFromCommand); + return self._postProcess( + argvPromise ?? argv, + populateDoubleDash, + !!calledFromCommand, + true + ); }; + // If argv is a promise (which is possible if async middleware is used) + // delay applying validation until the promise has resolved: + function validateAsync( + 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; + } + // Applies a couple post processing steps that are easier to perform // as a final step. self._postProcess = function ( argv: Arguments | Promise, populateDoubleDash: boolean, - calledFromCommand = false + calledFromCommand: boolean, + runGlobalMiddleware: boolean ): any { - if (isPromise(argv)) return argv; if (calledFromCommand) return argv; + if (isPromise(argv)) return argv; if (!populateDoubleDash) { argv = self._copyDoubleDash(argv); } @@ -1689,6 +1745,9 @@ function Yargs( if (parsePositionalNumbers) { argv = self._parsePositionalNumbers(argv); } + if (runGlobalMiddleware) { + argv = applyMiddleware(argv, self, globalMiddleware, false); + } return argv; }; @@ -1728,33 +1787,37 @@ function Yargs( }; self._runValidation = function runValidation( - argv, aliases, positionalMap, parseErrors, isDefaultCommand = false - ) { - if (parseErrors) throw new YError(parseErrors.message); - validation.nonOptionCount(argv); - validation.requiredArguments(argv); - let failedStrictCommands = false; - if (strictCommands) { - failedStrictCommands = validation.unknownCommands(argv); - } - if (strict && !failedStrictCommands) { - validation.unknownArguments( - argv, - aliases, - positionalMap, - isDefaultCommand - ); - } else if (strictOptions) { - validation.unknownArguments(argv, aliases, {}, false, false); - } - validation.customChecks(argv, aliases); - validation.limitedChoices(argv); - validation.implications(argv); - validation.conflicting(argv); + ): (argv: Arguments) => void { + aliases = {...aliases}; + positionalMap = {...positionalMap}; + const demandedOptions = {...self.getDemandedOptions()}; + return (argv: Arguments) => { + if (parseErrors) throw new YError(parseErrors.message); + validation.nonOptionCount(argv); + validation.requiredArguments(argv, demandedOptions); + let failedStrictCommands = false; + if (strictCommands) { + failedStrictCommands = validation.unknownCommands(argv); + } + if (strict && !failedStrictCommands) { + validation.unknownArguments( + argv, + aliases, + positionalMap, + isDefaultCommand + ); + } else if (strictOptions) { + validation.unknownArguments(argv, aliases, {}, false, false); + } + validation.customChecks(argv, aliases); + validation.limitedChoices(argv); + validation.implications(argv); + validation.conflicting(argv); + }; }; function guessLocale() { @@ -1775,6 +1838,7 @@ function Yargs( return self; } + // rebase an absolute path to a relative one with respect to a base directory // exported for tests export interface RebaseFunction { @@ -1795,11 +1859,11 @@ export interface YargsInstance { _postProcess>( argv: T, populateDoubleDash: boolean, - calledFromCommand?: boolean + calledFromCommand: boolean, + runGlobalMiddleware: boolean ): T; _copyDoubleDash(argv: T): T; _parsePositionalNumbers(argv: T): T; - _getLoggerInstance(): LoggerInstance; _getParseContext(): Object; _hasOutput(): boolean; @@ -1808,7 +1872,7 @@ export interface YargsInstance { ( args: string | string[] | null, shortCircuit?: boolean, - _calledFromCommand?: boolean, + calledFromCommand?: boolean, commandIndex?: number, helpOnly?: boolean ): Arguments | Promise; @@ -1817,12 +1881,11 @@ export interface YargsInstance { | Promise; }; _runValidation( - argv: Arguments, aliases: Dictionary, positionalMap: Dictionary, parseErrors: Error | null, isDefaultCommand?: boolean - ): void; + ): (argv: Arguments) => void; _setHasOutput(): void; addHelpOpt: { (opt?: string | false): YargsInstance; diff --git a/test/middleware.cjs b/test/middleware.cjs index 0c779220c..4efe10300 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -117,136 +117,6 @@ describe('middleware', () => { .parse(); }); - // addresses https://github.com/yargs/yargs/issues/1237 - describe('async', () => { - it('fails when the promise returned by the middleware rejects', done => { - const error = new Error(); - const handlerErr = new Error('should not have been called'); - yargs('foo') - .command( - 'foo', - 'foo command', - () => {}, - argv => done(handlerErr), - [argv => Promise.reject(error)] - ) - .fail((msg, err) => { - expect(msg).to.equal(null); - expect(err).to.equal(error); - done(); - }) - .parse(); - }); - - it('it allows middleware rejection to be caught', async () => { - const argvPromise = yargs('foo') - .command( - 'foo', - 'foo command', - () => {}, - () => {} - ) - .middleware(async () => { - return new Promise((resolve, reject) => { - setTimeout(() => { - return reject(Error('error from middleware')); - }, 5); - }); - }) - .fail(false) - .parse(); - try { - await argvPromise; - throw Error('unreachable'); - } catch (err) { - err.message.should.match(/error from middleware/); - } - }); - - it('it awaits middleware before awaiting handler, when applyBeforeValidation is "false"', async () => { - let log = ''; - const argvPromise = yargs('foo --bar') - .command( - 'foo', - 'foo command', - () => {}, - async () => { - return new Promise(resolve => { - setTimeout(() => { - log += 'handler'; - return resolve(); - }, 5); - }); - } - ) - .middleware(async argv => { - return new Promise(resolve => { - setTimeout(() => { - log += 'middleware'; - argv.fromMiddleware = 99; - return resolve(); - }, 20); - }); - }, false) - .parse(); - const argv = await argvPromise; - log.should.equal('middlewarehandler'); - argv.fromMiddleware.should.equal(99); - argv.bar.should.equal(true); - }); - - it('calls the command handler when all middleware promises resolve', done => { - const middleware = (key, value) => () => - new Promise((resolve, reject) => { - setTimeout(() => { - return resolve({[key]: value}); - }, 5); - }); - yargs('foo hello') - .command( - 'foo ', - 'foo command', - () => {}, - argv => { - argv.hello.should.equal('world'); - argv.foo.should.equal('bar'); - done(); - }, - [middleware('hello', 'world'), middleware('foo', 'bar')] - ) - .fail((msg, err) => { - return done(Error('should not have been called')); - }) - .exitProcess(false) - .parse(); - }); - - it('calls an async middleware only once for nested subcommands', done => { - let callCount = 0; - const argv = yargs('cmd subcmd') - .command('cmd', 'cmd command', yargs => { - yargs.command('subcmd', 'subcmd command', yargs => {}); - }) - .middleware( - argv => - new Promise(resolve => { - callCount++; - resolve(argv); - }) - ) - .parse(); - - if (!(argv instanceof Promise)) done('argv should be a Promise'); - - argv - .then(() => { - callCount.should.equal(1); - done(); - }) - .catch(err => done(err)); - }); - }); - // see: https://github.com/yargs/yargs/issues/1281 it("doesn't modify globalMiddleware array when executing middleware", () => { let count = 0; @@ -370,37 +240,62 @@ describe('middleware', () => { .parse(); }); - it('throws an error if promise returned and applyBeforeValidation enabled', () => { - expect(() => { - yargs(['mw']) + it('resolves async middleware, before applying validation', async () => { + const argv = await yargs(['mw']) + .fail(false) + .middleware( + [ + async function (argv) { + return new Promise(resolve => { + setTimeout(() => { + argv.mw = 'mw'; + argv.other = true; + return resolve(argv); + }, 5); + }); + }, + ], + true + ) + .command('mw', 'adds func to middleware', { + mw: { + demand: true, + string: true, + }, + }) + .parse(); + argv.other.should.equal(true); + argv.mw.should.equal('mw'); + }); + + it('still throws error when async middleware is used', async () => { + try { + const argv = await yargs(['mw']) + .fail(false) .middleware( [ - function (argv) { - argv.mw = 'mw'; - argv.other = true; - return Promise.resolve(argv); + async function (argv) { + return new Promise(resolve => { + setTimeout(() => { + argv.other = true; + return resolve(argv); + }, 5); + }); }, ], true ) - .command( - 'mw', - 'adds func to middleware', - { - mw: { - demand: true, - string: true, - }, + .command('mw', 'adds func to middleware', { + mw: { + demand: true, + string: true, }, - argv => { - throw Error('we should not get here'); - } - ) - .exitProcess(false) + }) .parse(); - }).to.throw( - 'middleware cannot return a promise when applyBeforeValidation is true' - ); + throw Error('unreachable'); + } catch (err) { + err.message.should.match(/Missing required argument/); + } }); it('runs before validation, when middleware is added in builder', done => { @@ -472,4 +367,259 @@ describe('middleware', () => { .parse(); }); }); + + // addresses https://github.com/yargs/yargs/issues/1237 + describe('async', () => { + it('fails when the promise returned by the middleware rejects', done => { + const error = new Error(); + const handlerErr = new Error('should not have been called'); + yargs('foo') + .command( + 'foo', + 'foo command', + () => {}, + argv => done(handlerErr), + [argv => Promise.reject(error)] + ) + .fail((msg, err) => { + expect(msg).to.equal(null); + expect(err).to.equal(error); + done(); + }) + .parse(); + }); + + it('it allows middleware rejection to be caught', async () => { + const argvPromise = yargs('foo') + .command( + 'foo', + 'foo command', + () => {}, + () => {} + ) + .middleware(async () => { + return new Promise((resolve, reject) => { + setTimeout(() => { + return reject(Error('error from middleware')); + }, 5); + }); + }) + .fail(false) + .parse(); + try { + await argvPromise; + throw Error('unreachable'); + } catch (err) { + err.message.should.match(/error from middleware/); + } + }); + + it('it awaits middleware before awaiting handler, when applyBeforeValidation is "false"', async () => { + let log = ''; + const argvPromise = yargs('foo --bar') + .command( + 'foo', + 'foo command', + () => {}, + async () => { + return new Promise(resolve => { + setTimeout(() => { + log += 'handler'; + return resolve(); + }, 5); + }); + } + ) + .middleware(async argv => { + return new Promise(resolve => { + setTimeout(() => { + log += 'middleware'; + argv.fromMiddleware = 99; + return resolve(); + }, 20); + }); + }, false) + .parse(); + const argv = await argvPromise; + log.should.equal('middlewarehandler'); + argv.fromMiddleware.should.equal(99); + argv.bar.should.equal(true); + }); + + it('calls the command handler when all middleware promises resolve', done => { + const middleware = (key, value) => () => + new Promise((resolve, reject) => { + setTimeout(() => { + return resolve({[key]: value}); + }, 5); + }); + yargs('foo hello') + .command( + 'foo ', + 'foo command', + () => {}, + argv => { + argv.hello.should.equal('world'); + argv.foo.should.equal('bar'); + done(); + }, + [middleware('hello', 'world'), middleware('foo', 'bar')] + ) + .fail((msg, err) => { + return done(Error('should not have been called')); + }) + .exitProcess(false) + .parse(); + }); + + it('calls an async middleware only once for nested subcommands', done => { + let callCount = 0; + const argv = yargs('cmd subcmd') + .command('cmd', 'cmd command', yargs => { + yargs.command('subcmd', 'subcmd command', yargs => {}); + }) + .middleware( + argv => + new Promise(resolve => { + callCount++; + resolve(argv); + }) + ) + .parse(); + + if (!(argv instanceof Promise)) done(Error('argv should be a Promise')); + + argv + .then(() => { + callCount.should.equal(1); + done(); + }) + .catch(err => done(err)); + }); + + describe('$0', () => { + it('applies global middleware when no commands are provided, with implied $0', async () => { + const argv = await yargs('--foo 99') + .middleware(argv => { + return new Promise(resolve => { + setTimeout(() => { + argv.foo = argv.foo * 3; + return resolve(); + }, 20); + }); + }) + .parse(); + argv.foo.should.equal(297); + }); + + it('applies middleware before performing validation, with implied $0', async () => { + const argvEventual = yargs('--foo 100') + .option('bar', { + demand: true, + }) + .middleware(async argv => { + return new Promise(resolve => { + setTimeout(() => { + argv.foo = argv.foo * 2; + argv.bar = 'hello'; + return resolve(); + }, 100); + }) + }, true) + .check((argv) => { + if (argv.foo > 100) return true; + else return false; + }) + .parse(); + const argv = await argvEventual; + argv.foo.should.equal(200); + argv.bar.should.equal('hello'); + }); + + it('applies middleware before performing validation, with explicit $0', async () => { + const argvEventual = yargs('--foo 100') + .usage( + '$0', + 'usage', + () => {}, + ) + .option('bar', { + demand: true, + }) + .middleware(async argv => { + return new Promise(resolve => { + setTimeout(() => { + argv.foo = argv.foo * 2; + argv.bar = 'hello'; + return resolve(); + }, 100); + }) + }, true) + .check((argv) => { + if (argv.foo > 100) return true; + else return false; + }) + .parse(); + const argv = await argvEventual; + argv.foo.should.equal(200); + argv.bar.should.equal('hello'); + }); + }); + }); + + describe('synchronous $0', () => { + it('applies global middleware when no commands are provided', () => { + const argv = yargs('--foo 99') + .middleware(argv => { + argv.foo = argv.foo * 2; + }) + .parse(); + argv.foo.should.equal(198); + }); + it('applies global middleware when default command is provided, with explicit $0', () => { + const argv = yargs('--foo 100') + .usage( + '$0', + 'usage', + () => {}, + argv => { + argv.foo = argv.foo * 3; + } + ) + .middleware(argv => { + argv.foo = argv.foo * 2; + }) + .parse(); + argv.foo.should.equal(600); + }); + it('applies middleware before performing validation, with implicit $0', () => { + const argv = yargs('--foo 100') + .option('bar', { + demand: true, + }) + .middleware(argv => { + argv.foo = argv.foo * 2; + argv.bar = 'hello' + }, true) + .check((argv) => { + if (argv.foo > 100) return true; + else return false; + return true; + }) + .parse(); + argv.foo.should.equal(200); + argv.bar.should.equal('hello'); + }); + }); + + // Refs: https://github.com/yargs/yargs/issues/1351 + it('should run even if no command is matched', () => { + const argv = yargs('snuh --foo 99') + .middleware(argv => { + argv.foo = argv.foo * 2; + }) + .command('bar', 'bar command', () => {}, () => {}) + .parse(); + argv.foo.should.equal(198); + }); });