From 210191308a15a4e1b12c0f0f48746b094b7575ff Mon Sep 17 00:00:00 2001 From: bcoe Date: Fri, 2 Apr 2021 17:32:56 -0700 Subject: [PATCH 1/2] fix(async): don't call parse callback until async ops complete --- lib/yargs-factory.ts | 32 ++++++++++++++++++++------ test/command.cjs | 14 +++++++++--- test/middleware.cjs | 2 +- test/yargs.cjs | 53 ++++++++++++++------------------------------ 4 files changed, 54 insertions(+), 47 deletions(-) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index d83767608..f9c81a404 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -1052,13 +1052,31 @@ export class YargsInstance { if (this.#parseFn) this.#exitProcess = false; - const parsed = this[kRunYargsParserAndExecuteCommands]( - args, - !!shortCircuit - ); + let parsed = this[kRunYargsParserAndExecuteCommands](args, !!shortCircuit); this.#completion!.setParsed(this.parsed as DetailedArguments); - if (this.#parseFn) this.#parseFn(this.#exitError, parsed, this.#output); - this[kUnfreeze](); // Pop the stack. + if (isPromise(parsed)) { + parsed = parsed + .then(argv => { + if (this.#parseFn) this.#parseFn(this.#exitError, argv, this.#output); + return argv; + }) + .catch(err => { + if (this.#parseFn) { + this.#parseFn!( + err, + (this.parsed as DetailedArguments).argv, + this.#output + ); + } + throw err; + }) + .finally(() => { + this[kUnfreeze](); // Pop the stack. + }); + } else { + if (this.#parseFn) this.#parseFn(this.#exitError, parsed, this.#output); + this[kUnfreeze](); // Pop the stack. + } return parsed; } parserConfiguration(config: Configuration) { @@ -2262,7 +2280,7 @@ interface FrozenYargsInstance { interface ParseCallback { ( err: YError | string | undefined | null, - argv: Arguments | Promise, + argv: Arguments, output: string ): void; } diff --git a/test/command.cjs b/test/command.cjs index 32787b42c..f514d4760 100644 --- a/test/command.cjs +++ b/test/command.cjs @@ -2054,9 +2054,17 @@ describe('Command', () => { assert.strictEqual(set, true); }); }); - // TODO: investigate why .parse('cmd --help', () => {}); does not - // work properly with an async builder. We should test the same - // with handler. + // Refs: https://github.com/yargs/yargs/issues/1894 + it('does not print to stdout when parse callback is provided', async () => { + await yargs() + .command('cmd ', 'a test command', async () => { + await wait(); + }) + .parse('cmd --help', (_err, argv, output) => { + output.should.include('a test command'); + argv.help.should.equal(true); + }); + }); }); describe('builder', () => { diff --git a/test/middleware.cjs b/test/middleware.cjs index c607cfcec..04664b701 100644 --- a/test/middleware.cjs +++ b/test/middleware.cjs @@ -857,7 +857,7 @@ describe('middleware', () => { choices: [10, 20, 30], }) .coerce('foo', async arg => { - wait(); + await wait(); return (arg *= 2); }) .parse('--foo 2'), diff --git a/test/yargs.cjs b/test/yargs.cjs index bb3806a94..1840fda46 100644 --- a/test/yargs.cjs +++ b/test/yargs.cjs @@ -2687,7 +2687,7 @@ describe('yargs dsl tests', () => { // See: https://github.com/yargs/yargs/issues/1420 describe('async', () => { describe('parse', () => { - it('returns promise that resolves argv on success', done => { + it('calls parse callback once async handler has resolved', done => { let executionCount = 0; yargs() .command( @@ -2705,15 +2705,13 @@ describe('yargs dsl tests', () => { } ) .parse('cmd foo', async (_err, argv) => { - (typeof argv.then).should.equal('function'); - argv = await argv; argv.addedAsync.should.equal(99); argv.str.should.equal('foo'); executionCount.should.equal(1); return done(); }); }); - it('returns deeply nested promise that resolves argv on success', done => { + it('calls parse callback once deeply nested promise has resolved', done => { let executionCount = 0; yargs() .command( @@ -2738,15 +2736,13 @@ describe('yargs dsl tests', () => { () => {} ) .parse('cmd foo orange', async (_err, argv) => { - (typeof argv.then).should.equal('function'); - argv = await argv; argv.addedAsync.should.equal(99); argv.apple.should.equal('orange'); executionCount.should.equal(1); return done(); }); }); - it('returns promise that can be caught if rejected', done => { + it('populates err with async rejection', done => { let executionCount = 0; yargs() .command( @@ -2762,15 +2758,10 @@ describe('yargs dsl tests', () => { }); } ) - .parse('cmd foo', async (_err, argv) => { - (typeof argv.then).should.equal('function'); - try { - await argv; - } catch (err) { - err.message.should.equal('async error'); - executionCount.should.equal(1); - return done(); - } + .parse('cmd foo', async err => { + err.message.should.equal('async error'); + executionCount.should.equal(1); + return done(); }); }); it('caches nested help output, so that it can be output by showHelp()', done => { @@ -2789,16 +2780,11 @@ describe('yargs dsl tests', () => { }); } ).parse('cmd foo', async (_err, argv) => { - (typeof argv.then).should.equal('function'); - try { - await argv; - } catch (err) { - y.showHelp(output => { - output.should.match(/a command/); - executionCount.should.equal(1); - return done(); - }); - } + y.showHelp(output => { + output.should.match(/a command/); + executionCount.should.equal(1); + return done(); + }); }); }); it('caches deeply nested help output, so that it can be output by showHelp()', done => { @@ -2824,16 +2810,11 @@ describe('yargs dsl tests', () => { }, () => {} ).parse('cmd inner bar', async (_err, argv) => { - (typeof argv.then).should.equal('function'); - try { - await argv; - } catch (err) { - y.showHelp(output => { - output.should.match(/inner command/); - executionCount.should.equal(1); - return done(); - }); - } + y.showHelp(output => { + output.should.match(/inner command/); + executionCount.should.equal(1); + return done(); + }); }); }); }); From 37e80413c074983ffb02e6a60f5f7bda6fa78bd4 Mon Sep 17 00:00:00 2001 From: bcoe Date: Fri, 2 Apr 2021 17:41:48 -0700 Subject: [PATCH 2/2] refactor: switch back to using const --- lib/yargs-factory.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/yargs-factory.ts b/lib/yargs-factory.ts index f9c81a404..54c1c3ba8 100644 --- a/lib/yargs-factory.ts +++ b/lib/yargs-factory.ts @@ -1052,10 +1052,13 @@ export class YargsInstance { if (this.#parseFn) this.#exitProcess = false; - let parsed = this[kRunYargsParserAndExecuteCommands](args, !!shortCircuit); + const parsed = this[kRunYargsParserAndExecuteCommands]( + args, + !!shortCircuit + ); this.#completion!.setParsed(this.parsed as DetailedArguments); if (isPromise(parsed)) { - parsed = parsed + return parsed .then(argv => { if (this.#parseFn) this.#parseFn(this.#exitError, argv, this.#output); return argv;