Opt-in behaviour to avoid name pollution

[shadowspawn]

Pull Request

Problem

Commander has traditionally stored the option values on the command object. This gives a compact syntax especially when working with the program object, but means there are potential naming conflicts between user defined options and properties on Command. This catches people out and there are not good work-arounds.

Collected issues: #933

For example, code with a conflict on both program and a command::

const commander = require('commander');

const program = new commander.Command();
program
  .option('--name <value>', 'specify name');
program
  .command('sub')
  .option('--action <value>', 'specify action')
  .action((options) => {
    console.log(options.action);
  });

program.parse(process.argv);

options = program.opts();
console.log(options.name);

% node pollution.js sub
[Function]
[Function]
% node pollution.js --name NAME sub --action ACTION
ACTION
NAME

Solution

Add a way of configuring Command to behave differently. Implement "modern" configuration which stores the option values separately, and passes just the options to action handlers. The main code change is accessing program options through (existing) .opts() member.

• backwards compatible. Existing behaviour unchanged.
• often no changes required in action handlers. Most people probably think of the parameter as for just the options anyway!
• configurable for special cases or existing advanced usage requiring command object passed to action handler

With the addition of couple of lines to the example program above, no other code changes:

...
const program = new commander.Command();
program
  .storeOptionsAsProperties(false)
  .passCommandToAction(false);
...

Now get:

% node pollution.js sub                            
undefined
undefined
% node pollution.js --name NAME sub --action ACTION
ACTION
NAME

ToDo

Add info to README.

Notes

I considered using a Map for the option values rather than a plain Object, but it would mean changing all existing client code accessing option values. Continuing to use an object means action handling code is largely unaffected.

I considered creating option values object with Object.create(null) to further reduce name collisions, but felt it was too unusual for the small benefits.

I would have exposed a data property called opts if there was not already a function with that name! However, using the existing function does allow code that works with both the old way and the new way, which is nice.

We could change all the code examples to use program.opts() so they work with both old and modern behaviour.

In a future major version we could change the default behaviour. The routines are named for the future, where you need to opt back into the old behaviour.

[shadowspawn]

Old behaviour:

const program = new commander.Command();

New behaviour, safe options, which are passed to action handler:

const program = new commander.Command();
program
   .storeOptionsAsProperties(false)
   .passCommandToAction(false);

(The routines are named for the future when the default behaviour has changed, and would pass true to get the legacy behaviour.)

[shadowspawn]

Added section in README covering new routines and rationale, and example files for the three expected code patterns.

Ready for feedback/review.

[abetomo]

LGTM :-)

[shadowspawn]

Woo hoo! 🎉

[shadowspawn]

I am thinking perhaps do next release in early January?

(I am starting to get busy leading into Xmas so my time will be limited.)

[shadowspawn]

Shipped in Commander v4.1