fix option type definition to match usage.

[UrielCh]

Pull Request

  npm run test
Test Suites: 32 passed, 32 total
Tests:       223 passed, 223 total
Snapshots:   0 total
Time:        3.109s
Ran all test suites.

npm run lint
no changes.

Problem

// I want to use this kind of option
    .option('--hostname <hostname>', 'hostname used for binding http server', (value: string, previous: string[]) => { previous.push(value); }, [] as string[])
// currently the typing using any, any, any
// argument are named arg1, arg2
// now it's value, previous

Solution

• enforce typings of option.
• add some extra typing test.

ChangeLog

• improve option() typing enforcing function usage.

[shadowspawn]

I agree, the current typing does not offer much help for intended usage!

[shadowspawn]

I am still thinking about whether it is ok to try and enforce that the previous, the return type, and the default value are all the same type. I think that will often be the case.

[UrielCh]

requiredOption is now also updated.
Function Overloads order updated.

[UrielCh]

so If wee Keep the actual implementation, I can add this to the tests :

function collectVoid(value, previous) {
  previous.concat([value]);
}

...

test('when collect returning Void -c a -c b -c c then value is undefigned', () => {
  const program = new commander.Command();
  program
    .option('-c, --collect <value>', 'repeatable value', collectVoid, []);
  program.parse(['node', 'test', '-c', 'a', '-c', 'b', '-c', 'c']);
  expect(program.collect).toBeUndefined();
});

test('when collect returning Void -c a -c b -c c -c d then value is []', () => {
  const program = new commander.Command();
  program
    .option('-c, --collect <value>', 'repeatable value', collectVoid, []);
  program.parse(['node', 'test', '-c', 'a', '-c', 'b', '-c', 'c', '-c', 'd']);
  expect(program.collect).toEqual([]);
});

[UrielCh]

new version enforcing current implementation.
remove void returning collector from typing.
force string type as default value is no conversion function is given

[shadowspawn]

I'll be looking to include these changes in release/5.x branch, but I won't change the merge target branch until it has passed review in case it mucks up the code comments or causes merge conflicts which I can sort out later.

[shadowspawn]

Thanks for your enthusiasm in improving the types! It has taken some digging to cleanly describe the intended usage patterns.

[shadowspawn]

This does now describe the intended uses much better and in a more helpful way. It is more opinionated than typical for Commander, and people may report reasonable uses we have not thought of! In that case, we can change or weaken the typing as we learn more.

[shadowspawn]

(Don't worry about the merge conflict, I will resolve that by hand when has passed second review. In comments rather than in code.)

[shadowspawn]

This PR is included in the published pre-release of Commander v5.0.0 (#1163)

[shadowspawn]

Commander v5.0.0 has been released.

https://github.com/tj/commander.js/releases/tag/v5.0.0