Skip to content

Commit 2d1136d

Browse files
authored
fix: coerce pollutes argv (#2161)
1 parent ad9fcac commit 2d1136d

File tree

3 files changed

+67
-4
lines changed

3 files changed

+67
-4
lines changed

lib/command.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -603,9 +603,9 @@ export class CommandInstance {
603603
// If both positionals/options provided, no default was set,
604604
// and if at least one is an array: don't overwrite, combine.
605605
if (
606-
!Object.prototype.hasOwnProperty.call(defaults, key) &&
607-
Object.prototype.hasOwnProperty.call(argv, key) &&
608-
Object.prototype.hasOwnProperty.call(parsed.argv, key) &&
606+
!Object.hasOwnProperty.call(defaults, key) &&
607+
Object.hasOwnProperty.call(argv, key) &&
608+
Object.hasOwnProperty.call(parsed.argv, key) &&
609609
(Array.isArray(argv[key]) || Array.isArray(parsed.argv[key]))
610610
) {
611611
argv[key] = ([] as string[]).concat(argv[key], parsed.argv[key]);

lib/yargs-factory.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,13 @@ export class YargsInstance {
387387
yargs: YargsInstance
388388
): Partial<Arguments> | Promise<Partial<Arguments>> => {
389389
let aliases: Dictionary<string[]>;
390+
391+
// Skip coerce logic if related arg was not provided
392+
const shouldCoerce = Object.hasOwnProperty.call(argv, keys);
393+
if (!shouldCoerce) {
394+
return argv;
395+
}
396+
390397
return maybeAsyncResult<
391398
Partial<Arguments> | Promise<Partial<Arguments>> | any
392399
>(
@@ -396,7 +403,10 @@ export class YargsInstance {
396403
},
397404
(result: any): Partial<Arguments> => {
398405
argv[keys] = result;
399-
if (aliases[keys]) {
406+
const stripAliased = yargs
407+
.getInternalMethods()
408+
.getParserConfiguration()['strip-aliased'];
409+
if (aliases[keys] && stripAliased !== true) {
400410
for (const alias of aliases[keys]) {
401411
argv[alias] = result;
402412
}

test/command.cjs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1647,6 +1647,59 @@ describe('Command', () => {
16471647
argv.rest.should.equal('bar baz');
16481648
coerceExecutionCount.should.equal(1);
16491649
});
1650+
1651+
// Addresses: https://github.com/yargs/yargs/issues/2130
1652+
it('should not run or set new properties on argv when related argument is not passed', () => {
1653+
yargs('cmd1')
1654+
.command(
1655+
'cmd1',
1656+
'cmd1 desc',
1657+
yargs =>
1658+
yargs
1659+
.option('foo', {alias: 'f', type: 'string'})
1660+
.option('bar', {
1661+
alias: 'b',
1662+
type: 'string',
1663+
implies: 'f',
1664+
coerce: () => expect.fail(), // Should not be called
1665+
})
1666+
.fail(() => {
1667+
expect.fail(); // Should not fail because of implies
1668+
}),
1669+
argv => {
1670+
// eslint-disable-next-line no-prototype-builtins
1671+
if (Object.prototype.hasOwnProperty(argv, 'b')) {
1672+
expect.fail(); // 'b' was not provided, coerce should not set it
1673+
}
1674+
}
1675+
)
1676+
.strict()
1677+
.parse();
1678+
});
1679+
1680+
// Addresses: https://github.com/yargs/yargs/issues/2159
1681+
it('should not add aliases to argv if strip-aliased config is true', () => {
1682+
yargs('cmd1 -f hello -b world')
1683+
.parserConfiguration({'strip-aliased': true})
1684+
.command(
1685+
'cmd1',
1686+
'cmd1 desc',
1687+
yargs =>
1688+
yargs.option('foo', {alias: 'f', type: 'string'}).option('bar', {
1689+
type: 'string',
1690+
alias: 'b',
1691+
coerce: val => val,
1692+
}),
1693+
argv => {
1694+
// eslint-disable-next-line no-prototype-builtins
1695+
if (Object.prototype.hasOwnProperty(argv, 'b')) {
1696+
expect.fail(); // 'b' is an alias, it should not be in argv
1697+
}
1698+
}
1699+
)
1700+
.strict()
1701+
.parse();
1702+
});
16501703
});
16511704

16521705
describe('defaults', () => {

0 commit comments

Comments
 (0)