chore(ts): tsify lib/command #1654
Conversation
|
To be rebased before review, once #1636 is reviewed and merged. |
mleguen
force-pushed
the
1586-ts-command
branch
3 times, most recently
from
7ec5e9b
to
298e4fd
Compare
The documentation states addHandler can be called either with a string description or false to disable description display. But true was also silently accepted, which led to displaying 'true' as the description.
ParseCommand filled options.alias for positionals with a string instead of a string[] as does yargs, so only the last defined alias was provided to yargs-parser and other aliases were silently ignored.
To avoid it to be seen as uncovered branches
| export type Dictionary<T = any> = { [key: string]: T } | ||
|
|
||
| export type NotEmptyArray<T = any> = [T, ...T[]] | ||
|
|
There was a problem hiding this comment.
is the idea that these types aren't specific to yargs data structures?
There was a problem hiding this comment.
Yes. The main reason for this is split() signature is not precise enough: it returns string[] as if the first argument could be undefined, which is not the case. Its real signature should be [string, ...string[]].
When you are destructuring the results of split(), for example:
const [cmd, ...args] = something.split();then cmd is string | undefined when it should be string.
There was a problem hiding this comment.
To answer more precisely your question: common-types was at first the place to put types several modules would use, but at the moment is mostly used by types which I think should be typescript standards, but are not yet implemented.
| _hasParseCallback (): boolean | ||
| // TODO: to be precised once yargs is tsified | ||
| _parseArgs (args: null, shortCircuit: null, _calledFromCommand: boolean, commandIndex: number): Arguments |
There was a problem hiding this comment.
when we make the next breaking change of yargs, should we migrate any of these _ methods to being private in TypeScript?
There was a problem hiding this comment.
Sure.
The next breaking change in yargs should be to switch from factories to classes, and use private and public methods to distinguish inner and API methods.
This would ease a lot understanding, IDE tools use, type inference, etc.
|
@mleguen a couple questions, I'm quite happy with this though 👍 |
Follow up (and last PR) of the 2nd step of #1586: this PR tsifies the lib/middleware module and fixes some bugs revealed by type errors:
truewas silently accepted as a command description, leading to displaying "true" as the descriptionoptions.aliaseswas wrongly used asstringinstead ofstring[]inparseCommand, leading to the last positional alias only to be taken into account