-
-
Notifications
You must be signed in to change notification settings - Fork 262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix ParamsFrom
type inference
#2108
Conversation
@@ -22,7 +22,7 @@ export class HelloCliTest extends CLI { | |||
}, | |||
}, | |||
countries: { | |||
variadic: true as const, | |||
variadic: true as true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to consolidate the Params type, but as const
or as true
is still required since typescript can't narrow the type well enough itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What we could also do is make a function called VariadicType that just takes an Input and says what our type is doing. Then we could avoid the as const
part.
Oh, this is a nice improvement to |
CLIParamsFrom
to its own typeParamsFrom
type inference
|
||
type Variadic = { variadic: true }; | ||
|
||
export type ParamsFrom<A extends Action | Task | CLI> = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth putting Action | Task | CLI
into a single type, but I don't think this is likely to change.
Releasing a new version now |
For some reason, TypeScript has trouble inferring types properly when using
A extends CLI
as a conditional.You can play around with this example here eee6e8d
Separating out ParamsFrom for the CLI to it's own type lets you explicitly use it, though I'd still like to find a solution that keeps things in a single
ParamsFrom
that works for both if possible, hence the draft PR. Pushed here for visibility.