Skip to content

Added TypeScript types #96

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

Merged
merged 2 commits into from
Sep 5, 2018
Merged

Conversation

DigitalFlow
Copy link
Contributor

Hey @jakezatecky,

thanks for your library, it's working really well for me.

I'm using TypeScript and wrote a type definition file for it.
Other TypeScript users will need this as well for using your library.

Would you be willing to include it in your repository?
If not I could also submit it to @types.

Kind Regards,
Florian

@jakezatecky
Copy link
Owner

I am certainly open to including the TypeScript definitions in the project, but I have never used TypeScript so I fear that the definitions might become stale overtime. From what I see in this file, it looks like I could update it myself (provided I remember) because the definitions look pretty straightforward.

A couple things for the current PR:

  • It looks like the definition for onClick says that the parameter supplied to the function will be a string. That function is actually passed an object with the value and checked properties. It looks like an interface would have to be defined for that object and the definition would need to be updated.
  • For consistency's sake, I would prefer that the opening braces not be on their own separate lines, just so this file aligns with the formatting for the rest of the project.

The build error you can ignore, as that has been fixed in master.

This PR also made me realize that I am missing a couple properties in the current node shape for the React prop types, so thanks for that.

@DigitalFlow
Copy link
Contributor Author

Hi Jake,

thanks for your reply.
I adjusted the brace spacing and the onClick parameters.
In addition to that, I removed the separate types folder, as it seemed unnecessary in retrospective.

Kind Regards,
Florian

@jakezatecky jakezatecky merged commit 559cd20 into jakezatecky:master Sep 5, 2018
@jakezatecky
Copy link
Owner

I went ahead and added @types/react to resolve some annoying errors in my IDE. I also updated the file to reflect the latest properties.

@DigitalFlow
Copy link
Contributor Author

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants