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

Feature request: built-in TypeScript type declarations #993

Closed
taeh98 opened this issue Apr 7, 2021 · 10 comments
Closed

Feature request: built-in TypeScript type declarations #993

taeh98 opened this issue Apr 7, 2021 · 10 comments

Comments

@taeh98
Copy link
Contributor

taeh98 commented Apr 7, 2021

Hello,

Please could you add built-in type declarations for TypeScript? This would allow this library to be used in TypeScript projects far more easily. At the moment, you have to use a third party DefinitelyTyped library to get type definitions for this library. This makes the type definitions less maintainable: when updated versions of this library are released, the third party library will inevitably lag behind. This adds the overhead of updating and matching the versions of these two libraries to TypeScript projects.

I think the steps to implement this would just be porting index.js to index.ts, though using TypeScript throughout this library would enable strict typing and compile time checks that could improve maintainability.

I think these issues and this pull request are related, but they seem stale - hence me opening this issue:

Thank you!

@mikehardy
Copy link
Collaborator

Sounds like a great idea for a PR!

@taeh98
Copy link
Contributor Author

taeh98 commented Apr 7, 2021

Sounds like a great idea for a PR!

I would happily do it myself, but the build processes make the port non-trivial. I think someone who is already familiar with the library should do this.

@mikehardy
Copy link
Collaborator

Best way to get oriented, do a PR ;-). Most react-native modules that do this are using 'bob', for example https://github.com/react-native-device-info/react-native-device-info/pull/799/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

(although bob lives here now https://github.com/callstack/react-native-builder-bob)

Sincerely, it's no big deal, getting the build together - using bob - should not be a hurdle of any sort

@MateusAndrade MateusAndrade pinned this issue Apr 7, 2021
@MateusAndrade
Copy link
Collaborator

Nice @taeh98, I think we can start as @mikehardy suggested. What do you think about placing the types from definitely-typed inside this library? It would be an easy and good start. 🚀

@taeh98
Copy link
Contributor Author

taeh98 commented Apr 7, 2021

Nice @taeh98, I think we can start as @mikehardy suggested. What do you think about placing the types from definitely-typed inside this library? It would be an easy and good start. rocket

Yeah I'm able to do that. Done in #994

@taeh98
Copy link
Contributor Author

taeh98 commented Apr 8, 2021

Nice @taeh98, I think we can start as @mikehardy suggested. What do you think about placing the types from definitely-typed inside this library? It would be an easy and good start. rocket

Yeah I'm able to do that. Done in #994

Now that the issues with the pull request have been fixed and it's gone through, please could you publish a new release? Just because I found this issue in the first place when trying to use this library in a project I'm making with TypeScript, so it would be really helpful. Thanks again!

@mikehardy
Copy link
Collaborator

@MateusAndrade you committed it with a chore label so semantic release did not launch it 😅 - but mostly I'm dropping back by to say @taeh98 champion! having types in here is fantastic, thank you

@MateusAndrade
Copy link
Collaborator

@MateusAndrade you committed it with a chore label so semantic release did not launch it 😅 - but mostly I'm dropping back by to say @taeh98 champion! having types in here is fantastic, thank you

Yes! I forget about that, I'm going to push a commit to force a fix.

@MateusAndrade
Copy link
Collaborator

@taeh98
Copy link
Contributor Author

taeh98 commented Apr 9, 2021

Here we go https://github.com/react-native-share/react-native-share/releases/tag/v5.2.0! eyes

Can we close this @taeh98 ?

Yes! Thanks for all your help :)

@taeh98 taeh98 closed this as completed Apr 9, 2021
@MateusAndrade MateusAndrade unpinned this issue Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants