-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Description
import { Option } from 'commander';
const option = new Option('--add <numbers...>')
.argParser((value, previous?: number) => {
return Number(previous) + Number(value);
})
.preset(123);
const prop = 'presetArg'; // since currently not declared in typings, see #1972
option.parseArg?.(option[prop] as Parameters<Option['preset']>[0], undefined);This results in:
error TS2345: Argument of type 'unknown' is not assignable to parameter of type 'string'.
The reason is that the Option.preset() parameter type is set to unknown,
commander.js/typings/index.d.ts
Line 118 in 4ef19fa
| preset(arg: unknown): this; |
while the parseArg type is designed in such a way that only strings are expected in the first argument.
commander.js/typings/index.d.ts
Line 97 in 4ef19fa
| parseArg?: <T>(value: string, previous: T) => T; |
This is problematic because custom processing (parseArg) is called for preset option values.
(The reason why the call in the library's source did not result in an error when running the type check that led to #1967 is that the .preset() parameter type is any rather than unknown in lib/option.js:
Lines 62 to 66 in 4ef19fa
| * @param {any} arg | |
| * @return {Option} | |
| */ | |
| preset(arg) { |
I suspect this discrepancy is due to an expectation that vanilla JSDoc supports any but not unknown, but actually, there seems to be only one way to declare parameters that accept any arguments in vanilla JSDoc, and that is @param {*}, so unknown could be used here just as well.)
But anyway. My suggestion is that we require that the argument to Option.preset() be a string. Or we change the parseArg signature so that arbitrary values are accepted.