-
Notifications
You must be signed in to change notification settings - Fork 66
Finishing the project commands TS port #1449
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
Conversation
I'm going to wait for this to be merged before I put this up for review. |
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.
Found a few minor TS issues but otherwise this is looking good!
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.
LGTM! Just a few nits
types/Yargs.ts
Outdated
export type YargsCommandModule<T, U> = Omit<CommandModule<T, U>, 'builder'> & { | ||
builder: (yargs: Argv) => Promise<Argv<U>>; | ||
}; |
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.
Nit: Since CommandModule
is an interface we can extend it and it might simplify the TS logic here.
export interface CommandModuleWithRequiredBuilder<T, U> extends CommandModule<T, U> {
builder: (yargs: Argv) => Promise<Argv<U>>
}
df00fca
Description and Context
This wraps up the work to port all of the project command files to TS. I had to make some yargs typing changes to make things work well. I'm trying to strike the balance between getting value out of the yargs types, and making this so clunky to use that it's too much of a hassle to follow the patterns.
I had to make a few changes:
makeYargsBuilder
util was incorrectly matching on command buckets for help output. For example, runninghs project create --help
was returning the help output forhs project --hlep
. I fixed this by matching on the last command that the user ran (e.g "create" inhs project create --help
)CommandModule
interface is a little annoying to use because it makes the builder optional, and changes the type from a simple function to something else. I created some overrides that simplifies that type for our use casesmakeYargsBuilder
nowScreenshots
TODO
Who to Notify