react-tooltip
react-tooltip copied to clipboard
Typescript Types & Other Clean Up
Hi,
See below a few ideas - let me know what you think...
Typescript Definitions What do the maintainers think about moving the typescript definitions for react-tooltip to this repo?
There are definitions on definitely typed, but it looks like they're not being updated in-sync with changes here, and including the definitions in the repo would ensure new releases include up-to-date type definitions.
If you're open to this, I'm happy to migrate / update the type defs.
Other Stuff I'm saying this as someone who recently tried to submit a pull request for this project - not as a criticism.
It was fairly tedious getting the project set up on my development machine (ex: the eslint rules are out of date, so they didn't run properly). I think we need a more explicit, step-by-step guide to project setup to make it truly idiot-proof (for folks like me). This will:
- Make it easier for folks to contribute
- Make it easier to test / maintain
Let me know if you have any thoughts. Thanks for all your good work.
Hi @Jarch09, would be great to migrate/update the type defs into the repo.
For the second one, yeah also should be updated the contributing readme, I would check the eslint rules.
Thanks for report it.
Please contribute any changes that you can to the readme, even if you need to include a qualifier, like: "You will probably need to...."
This tool was written by one person, @wwayne , and has been maintained by one or two people at a time since he decided to move on. I no longer work with React at all, so I am not keeping up with the react community and updates, so I can mainly offer suggestions and push buttons on this site. @Rogger794 is actively involved now as a maintainer, so we have some improvements being added, but any help active users can offer will make this a better tool!
For testing, ideally we should have some tests run automatically by Travis when PRs are submitted. Right now, I believe it just checks formatting and that the build succeeds.
Sounds good. I'll add the type defs these next couple days.
Can also do some work on cleaning up the project infrastructure (making it more idiot proof).
Thank you both for your help!
@aronhelser - no recent frontend work or using a different toolkit?
Both - last project was with Vue, current project is C++ desktop app. Thanks for your contributions!
On Tue, Mar 31, 2020 at 1:01 PM Jarch09 [email protected] wrote:
Sounds good. I'll add the type defs these next couple days.
Can also do some work on cleaning up the project infrastructure (making it more idiot proof).
Thank you both for your help!
@aronhelser https://github.com/aronhelser - no recent frontend work or using a different toolkit?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/wwayne/react-tooltip/issues/568#issuecomment-606752044, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEX6THPTQJYHGJDCLAJP4TTRKIOU3ANCNFSM4LXIDYKQ .
👍 This would be very helpful. I currently can't get this to compile in typescript because the arrowColor prop types is missing.
@aronhelser @Rogger794 - I was talking with a friend and we were thinking of porting this repo to Typescript if you're open to it.
Given the project has grown significantly in scope (more features / flexibility), I think Typescript would really improve the project's stability. Especially given the lack of a robust test suite (difficult to do here), I think this would be very helpful.
Are you guys open to this? We'd like to do this within the scope of the initial repo (since you and @wwayne have contributed so much), but if you prefer, we can create a separate repo and add you guys as contributors.
Also, if you're strongly opposed, I'm not trying to overstep. I am just a fan of Typescript and think it could be helpful here.
I agree to porting this repo to typescript, what is your opinión @aronhelser
I saw #571 merged, and tried to update and remove my dependency on DefinitelyTyped's @types/react-tooltip, but my project couldn't find the new react-tooltip.d.ts
file. I see that the new file isn't in the dist/
folder. Did the new types make it into the published package?
I doubt I will be doing any active development, so it's fine with me. Any users of the library are not required to use typescript, correct?
On Tue, Apr 7, 2020 at 11:10 PM Rogger794 [email protected] wrote:
I agree to porting this repo to typescript, what is your opinión @aronhelser https://github.com/aronhelser
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/wwayne/react-tooltip/issues/568#issuecomment-610727139, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEX6THO6O2R5O6BLOF7NLXTRLPTJLANCNFSM4LXIDYKQ .
@aronhelser - correct. It will be compiled down to good ol' Javascript, and can be used by everyone.
@Rogger794 - sounds good. We'll probably get started in the next week or so, and I'll post in this thread with updates / questions.
@tjangel - let's catch up offline
#579 is merged which added the types:
field to package.json
, but the definitions file still isn't in the published package as of 4.2.3. Perhaps it's because package.json
has files: ["dist"]
, but the definitions file isn't in dist/
?
Works as of 4.2.4. Thanks! PR to remove it from DefinitelyTyped: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/44079
With the newly added types file, there is no way to explicitly import the props to be used for extending usage in styled components. The file in @types
while very outdated allowed this functionality. Please consider updating the types file to be structured similarly to that or explicitly export more than just the default.
@rathpc I'm guessing your comment is a bit stale now, but for anyone interested, it does seem like the props for the tooltip are exported in the .d.ts
file.
This works
import ReactTooltip, { TooltipProps } from "react-tooltip";
Hi guys, closing due to age, we did some publishes recently, please try the latest one and if necessary, open a new issue, thanks!