WIP: Rework action parameters for more stable interface

[shadowspawn]

Pull Request

Trying out an idea to improve code patterns now and in the future. Long description to work through the reasoning although the key code change is only a few lines of code!

Problem

In Commander v6 the action handler may get passed based on settings and unexpected args:

• expected args + command with options as properties
• expected args + options
• expected args + command without options as properties
• any of above + array of extra args

This has not worked out as nicely as hoped for moving to storing options safely!

1. When the options were always stored on the command and it was passed to action handler (as in older versions), the action handler gets a command parameter with everything on it. When the options are stored separately, we want it to be easy and convenient to access the options values, but still tidy to access the command.

Always always passing in command and calling cmd.opts() would only be one extra line of code, but would appear in most action handlers in most programs, and feels like paying for the increased safety.

2. There are now multiple variations for passed options/command, so implementations will vary depending on the settings.

3. There are now multiple variations for passed options/command, so hard to write examples that work whatever the settings. (This is a downside I notice as a maintainer!)

4. With the extra args only sometimes present, the parameters are not consistent so trickier to write a generic action handler. e.g. Extra arguments : document and add configuration to fail if they exist #1268 (comment)

Solution

Always pass the action handler:

• expected args + options + command

For backwards compatibility, the "options" might actually also be the command! The command still has options on it so the pattern still works.

This means code can access the options without an extra line of code in the common case, and easily access the command if needed without making configuration changes (just declare the parameter). The parameters are not affected by extra args so easier to write generic code if needed.

const program = require('commander');

program
  .command('sub [param]')
  .option('-d, --debug')
  .action((param, options, cmd) => {
    if (options.debug) {
      console.log(cmd.opts());
      console.log(cmd.args)
    }
  console.log(`Called sub with "${param}"`);
  });

program.parse();

$ node index.js sub --debug a b c
{ debug: true }
[ 'a', 'b', 'c' ]
Called sub with "a"

The one outlier might be if settings are .storeOptionsAsProperties(false) but .passCommandToAction(true) which some people will have adopted in Commander v6. For backwards compatibility will probably pass command + command to avoid breaking this, but will change the documentation so this is less likely in new code.

Related: CommandStrict to get all the recommended strict settings with latest best practices. #1404

ChangeLog

[shadowspawn]

On hold here, but being included in #1409