Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix(async): don't call parse callback until async ops complete (#1896)
If a callback is provided to parse() it will now be executed after all promises have resolved.

The cause of #1888, was that this[kUnfreeze](); was being called too soon, which set this.#parseFn to null.

Fixes #1888
  • Loading branch information
bcoe committed Apr 3, 2021
1 parent c872dfc commit a93f5ff
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 43 deletions.
27 changes: 24 additions & 3 deletions lib/yargs-factory.ts
Expand Up @@ -1057,8 +1057,29 @@ export class YargsInstance {
!!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)) {
return 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) {
Expand Down Expand Up @@ -2262,7 +2283,7 @@ interface FrozenYargsInstance {
interface ParseCallback {
(
err: YError | string | undefined | null,
argv: Arguments | Promise<Arguments>,
argv: Arguments,
output: string
): void;
}
Expand Down
14 changes: 11 additions & 3 deletions test/command.cjs
Expand Up @@ -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 <foo>', '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', () => {
Expand Down
2 changes: 1 addition & 1 deletion test/middleware.cjs
Expand Up @@ -857,7 +857,7 @@ describe('middleware', () => {
choices: [10, 20, 30],
})
.coerce('foo', async arg => {
wait();
await wait();
return (arg *= 2);
})
.parse('--foo 2'),
Expand Down
53 changes: 17 additions & 36 deletions test/yargs.cjs
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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 => {
Expand All @@ -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 => {
Expand All @@ -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();
});
});
});
});
Expand Down

0 comments on commit a93f5ff

Please sign in to comment.