react-tooltip icon indicating copy to clipboard operation
react-tooltip copied to clipboard

Typescript Types & Other Clean Up

Open Jarch09 opened this issue 4 years ago • 15 comments

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:

  1. Make it easier for folks to contribute
  2. Make it easier to test / maintain

Let me know if you have any thoughts. Thanks for all your good work.

Jarch09 avatar Mar 31 '20 05:03 Jarch09

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.

roggervalf avatar Mar 31 '20 13:03 roggervalf

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.

aronhelser avatar Mar 31 '20 14:03 aronhelser

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?

Jarch09 avatar Mar 31 '20 17:03 Jarch09

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 .

aronhelser avatar Mar 31 '20 18:03 aronhelser

👍 This would be very helpful. I currently can't get this to compile in typescript because the arrowColor prop types is missing.

vicky-holcomb avatar Apr 07 '20 17:04 vicky-holcomb

@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.

Jarch09 avatar Apr 08 '20 03:04 Jarch09

I agree to porting this repo to typescript, what is your opinión @aronhelser

roggervalf avatar Apr 08 '20 03:04 roggervalf

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?

skyqrose avatar Apr 08 '20 14:04 skyqrose

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 avatar Apr 08 '20 14:04 aronhelser

@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.

Jarch09 avatar Apr 08 '20 17:04 Jarch09

@tjangel - let's catch up offline

Jarch09 avatar Apr 08 '20 17:04 Jarch09

#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/?

skyqrose avatar Apr 13 '20 15:04 skyqrose

Works as of 4.2.4. Thanks! PR to remove it from DefinitelyTyped: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/44079

skyqrose avatar Apr 21 '20 12:04 skyqrose

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 avatar Apr 24 '20 15:04 rathpc

@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";

cgat avatar Dec 01 '21 13:12 cgat

Hi guys, closing due to age, we did some publishes recently, please try the latest one and if necessary, open a new issue, thanks!

danielbarion avatar Nov 09 '22 16:11 danielbarion