Skip to content
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

TypeScript support, allow missing index routes, and more #31

Merged
merged 2 commits into from
Aug 9, 2020

Conversation

karaggeorge
Copy link
Collaborator

Fixes #5
Fixes #6
Fixes #13
Fixes #23

Fixes part of #7

Overall updates:

  • Rework positional arguments
    • Proper display for required/optional args (<required> vs [optional])
    • Add support for variadic args ([variadic..])
    • Respect positionalArgs order. It used to be random (or the order the keys in propTypes)
    • Add some validation for positionalArgs
  • Fix the read-commands.js logic to ignore non-js files, not mess up deeply stacked commands
  • Don't require every sub-command to have an index.js. If not, a default message rendering the other available subcommands is shown when "index" route is called
  • Fix yargs sub-command logic
    • Nesting commands with different options, would add those options to the nested commands as well, now it's properly scoped
    • If a command is not defined, it would execute silently (no error), not it shows an appropriate list of other sub-commands
    • Fix for small-edge case where if a command does not accept positional args, but there are other available sub-commands, those are suggested, as the user probably miss-typed the sub-command (used to try and call the index route)
  • Better TypeScript support
  • Adds tests for new functionality

Note for TypeScript, I tried looking into generating args based on types instead of propTypes but I wasn't able to find something concrete with ast, since it can get really complex with something like:


type FirstProps = {
   text: string
}

type OtherProps = FirstProps & {
	someOtherArg: boolean
}

type Props = OtherProps

const Home = (props: Props) => <Text>{props.text}</Text>;

export default Home

If there's a good way to go about it I can add in a separate PR. From some research it seems like this might be useful: https://ts-morph.com/ but I wasn't able to get far (though I didn't spend much time on this)

Thank you for Pastel, I think it's an awesome way to develop complicated Ink APIs ❤️
Let me know if any updates are needed 😄

@vadimdemedes
Copy link
Owner

@karaggeorge Fantastic work, thank you so much!

@vadimdemedes vadimdemedes merged commit a35417b into vadimdemedes:master Aug 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants