Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
fix: coerce middleware should be applied once (#1978)
  • Loading branch information
jly36963 committed Jul 19, 2021
1 parent d2c121b commit 14bd6be
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 24 deletions.
32 changes: 15 additions & 17 deletions lib/command.ts
Expand Up @@ -224,29 +224,27 @@ export class CommandInstance {
helpOnly,
helpOrVersionSet
);
if (isPromise(builderResult)) {
return builderResult.then(result => {
return this.applyMiddlewareAndGetResult(
return isPromise(builderResult)
? builderResult.then(result =>
this.applyMiddlewareAndGetResult(
isDefaultCommand,
commandHandler,
result.innerArgv,
currentContext,
helpOnly,
result.aliases,
yargs
)
)
: this.applyMiddlewareAndGetResult(
isDefaultCommand,
commandHandler,
result.innerArgv,
builderResult.innerArgv,
currentContext,
helpOnly,
result.aliases,
builderResult.aliases,
yargs
);
});
} else {
return this.applyMiddlewareAndGetResult(
isDefaultCommand,
commandHandler,
builderResult.innerArgv,
currentContext,
helpOnly,
builderResult.aliases,
yargs
);
}
}
private applyBuilderUpdateUsageAndParse(
isDefaultCommand: boolean,
Expand Down
15 changes: 12 additions & 3 deletions lib/middleware.ts
Expand Up @@ -12,10 +12,11 @@ export class GlobalMiddleware {
addMiddleware(
callback: MiddlewareCallback | MiddlewareCallback[],
applyBeforeValidation: boolean,
global = true
global = true,
mutates = false
): YargsInstance {
argsert(
'<array|function> [boolean] [boolean]',
'<array|function> [boolean] [boolean] [boolean]',
[callback, applyBeforeValidation, global],
arguments.length
);
Expand All @@ -36,6 +37,7 @@ export class GlobalMiddleware {
const m = callback as Middleware;
m.applyBeforeValidation = applyBeforeValidation;
m.global = global;
m.mutates = mutates;
this.globalMiddleware.push(callback as Middleware);
}
return this.yargs;
Expand All @@ -53,7 +55,7 @@ export class GlobalMiddleware {
else return !toCheck.includes(m.option);
});
(callback as Middleware).option = option;
return this.addMiddleware(callback, true, true);
return this.addMiddleware(callback, true, true, true);
}
getMiddleware() {
return this.globalMiddleware;
Expand Down Expand Up @@ -92,6 +94,11 @@ export function applyMiddleware(
return acc;
}

if (middleware.mutates) {
if (middleware.applied) return acc;
middleware.applied = true;
}

if (isPromise(acc)) {
return acc
.then(initialObj =>
Expand Down Expand Up @@ -124,4 +131,6 @@ export interface Middleware extends MiddlewareCallback {
applyBeforeValidation: boolean;
global: boolean;
option?: string;
mutates?: boolean;
applied?: boolean;
}
7 changes: 3 additions & 4 deletions lib/yargs-factory.ts
Expand Up @@ -1156,11 +1156,10 @@ export class YargsInstance {
'alias',
];
opts = objFilter(opts, (k, v) => {
let accept = supportedOpts.indexOf(k) !== -1;
// type can be one of string|number|boolean.
if (k === 'type' && ['string', 'number', 'boolean'].indexOf(v) === -1)
accept = false;
return accept;
if (k === 'type' && !['string', 'number', 'boolean'].includes(v))
return false;
return supportedOpts.includes(k);
});

// copy over any settings that can be inferred from the command string.
Expand Down
29 changes: 29 additions & 0 deletions test/command.cjs
Expand Up @@ -1547,6 +1547,35 @@ describe('Command', () => {
return done();
});
});

// addresses https://github.com/yargs/yargs/issues/1966
it('should not be applied multiple times for nested commands', () => {
let coerceExecutionCount = 0;

const argv = yargs('cmd1 cmd2 foo bar baz')
.command('cmd1', 'cmd1 desc', yargs =>
yargs.command('cmd2 <positional1> <rest...>', 'cmd2 desc', yargs =>
yargs
.positional('rest', {
type: 'string',
coerce: arg => {
if (coerceExecutionCount) {
throw Error('coerce applied multiple times');
}
coerceExecutionCount++;
return arg.join(' ');
},
})
.fail(() => {
expect.fail();
})
)
)
.parse();

argv.rest.should.equal('bar baz');
coerceExecutionCount.should.equal(1);
});
});

describe('defaults', () => {
Expand Down

0 comments on commit 14bd6be

Please sign in to comment.