fix: positionals should not overwrite options

[jly36963]

Addresses: #1637

Problem

When executing the default command, if both positionals and options are provided under the same namespace, positionals will overwrite the options in argv. This doesn't happen if a command is provided.

yargs

Command provided

This will behave correctly. argv.foods will have both apples and carrots

yargs
  .command({
    command: "cmd [foods..]",
    desc: "cmd desc",
    builder: (yargs) =>
      yargs.positional("foods", {
        desc: "foods desc",
        type: "string",
      }),
    handler: (argv) => {
      console.log(argv);
    },
  })
  .help()
  .parse("cmd --foods apples carrots");

logs

// first time through parser (kRunYargsParserAndExecuteCommands)
{ args: 'cmd --foods apples carrots', config: { 'populate--': true } }
{ 'parsed.argv': { _: [ 'cmd', 'carrots' ], foods: 'apples' } }

// second time through parser
{ args: 'cmd --foods apples carrots', config: { 'populate--': true } }
{ 'parsed.argv': { _: [ 'cmd' ], foods: [ 'apples', 'carrots' ] } }

// argv in handler
{
  _: [ 'cmd' ],
  foods: [ 'apples', 'carrots' ],
  '$0': 'test-issue.js'
}

Default command

This will behave incorrectly. argv.foods will only have carrots.

yargs
  .command({
    command: "$0 [foods..]",
    desc: "default desc",
    builder: (yargs) =>
      yargs.positional("foods", {
        desc: "foods desc",
        type: "string",
      }),
    handler: (argv) => {
      console.log(argv);
    },
  })
  .help();
  .parse('--foods apples carrots')

logs

// first time through parser (kRunYargsParserAndExecuteCommands)
{ args: '--foods apples carrots', config: { 'populate--': true } }
{ 'parsed.argv': { _: [ 'carrots' ], foods: 'apples' } }

// second time through parser
{ args: '--foods apples carrots', config: { 'populate--': true } }
{ 'parsed.argv': { _: [ 'carrots' ], foods: 'apples' } }

// argv in handler
{
  _: [],
  foods: [ 'carrots' ],
  '$0': 'test-issue.js'
}

The logic in postProcessPositionals will cause the positionals (['carrots']) to override the option ('apples') in argv.

Solution

Check during postProcessPositionals if a particular key is already set on argv. If both positionals/options are provided and at least one is an array, combine rather than overwrite.

Questions

When both positional and option are provided, what should be the behavior?

• if variadic, combine?
• if single value, overwrite?

(This is the assumption I made with this fix.)

[bcoe]

if single value, overwrite?

I think for this edge case, this assumption seems right to me. Someone writing a command line should avoid a collision like this, for arguments they define. So, this is something that will most likely come up in the edge-case of someone passing an unexpected command.

[bcoe]

This fix looks great to me 👍 thank you.

[foolip]

Hi all!

It looks like this was a breaking change, at least for how we used yargs in MDN's browser-compat-data. I've worked around it here, which touches all the relevant code: mdn/browser-compat-data#11990

Note that I tried and failed to make only the last positional argument variadic, the default values are combined with the given value even then. This seems like a limitation in yargs now, albeit one that I could work around by not using an array type at all.

[jly36963]

@foolip I believe I understand the problem -- I'll work on a fix tonight, and I'll keep you posted