Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix async middleware #2164

Merged
merged 8 commits into from May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
86 changes: 63 additions & 23 deletions lib/command.ts
Expand Up @@ -364,34 +364,16 @@ export class CommandInstance {
pc.push(c);
return `$0 ${pc.join(' ')}`;
}
private applyMiddlewareAndGetResult(
private handleValidationAndGetResult(
isDefaultCommand: boolean,
commandHandler: CommandHandler,
innerArgv: Arguments | Promise<Arguments>,
currentContext: Context,
helpOnly: boolean,
aliases: Dictionary<string[]>,
yargs: YargsInstance
): Arguments | Promise<Arguments> {
let positionalMap: Dictionary<string[]> = {};
// If showHelp() or getHelp() is being run, we should not
// execute middleware or handlers (these may perform expensive operations
// like creating a DB connection).
if (helpOnly) return innerArgv;
if (!yargs.getInternalMethods().getHasOutput()) {
positionalMap = this.populatePositionals(
commandHandler,
innerArgv as Arguments,
currentContext,
yargs
);
}
const middlewares = this.globalMiddleware
.getMiddleware()
.slice(0)
.concat(commandHandler.middlewares);
innerArgv = applyMiddleware(innerArgv, yargs, middlewares, true);

yargs: YargsInstance,
middlewares: Middleware[],
positionalMap: Dictionary<string[]>
) {
// we apply validation post-hoc, so that custom
// checks get passed populated positional arguments.
if (!yargs.getInternalMethods().getHasOutput()) {
Expand Down Expand Up @@ -453,6 +435,64 @@ export class CommandInstance {

return innerArgv;
}
private applyMiddlewareAndGetResult(
isDefaultCommand: boolean,
commandHandler: CommandHandler,
innerArgv: Arguments,
currentContext: Context,
helpOnly: boolean,
aliases: Dictionary<string[]>,
yargs: YargsInstance
): Arguments | Promise<Arguments> {
let positionalMap: Dictionary<string[]> = {};
// If showHelp() or getHelp() is being run, we should not
// execute middleware or handlers (these may perform expensive operations
// like creating a DB connection).
if (helpOnly) return innerArgv;
if (!yargs.getInternalMethods().getHasOutput()) {
positionalMap = this.populatePositionals(
commandHandler,
innerArgv as Arguments,
currentContext,
yargs
);
}
const middlewares = this.globalMiddleware
.getMiddleware()
.slice(0)
.concat(commandHandler.middlewares);

const maybePromiseArgv = applyMiddleware(
innerArgv,
yargs,
middlewares,
true
);

return isPromise(maybePromiseArgv)
? maybePromiseArgv.then(resolvedInnerArgv =>
this.handleValidationAndGetResult(
isDefaultCommand,
commandHandler,
resolvedInnerArgv,
currentContext,
aliases,
yargs,
middlewares,
positionalMap
)
)
: this.handleValidationAndGetResult(
isDefaultCommand,
commandHandler,
maybePromiseArgv,
currentContext,
aliases,
yargs,
middlewares,
positionalMap
);
}
// transcribe all positional arguments "command <foo> <bar> [apple]"
// onto argv.
private populatePositionals(
Expand Down
20 changes: 20 additions & 0 deletions test/middleware.cjs
Expand Up @@ -281,6 +281,26 @@ describe('middleware', () => {
}
});

// Addresses: https://github.com/yargs/yargs/issues/2124
// This test will fail if result of async middleware is not handled like a promise
it('does not cause an unexpected error when async middleware and strict are both used', done => {
const input = 'cmd1';
yargs(input)
.command(
'cmd1',
'cmd1 desc',
yargs => yargs.middleware(async argv => argv, true),
_argv => {
done();
}
)
.fail(msg => {
done(new Error(msg));
})
.strict()
.parse();
});

it('runs before validation, when middleware is added in builder', done => {
yargs(['mw'])
.command(
Expand Down