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 Types & Other Clean Up #568

Closed
Jarch09 opened this issue Mar 31, 2020 · 16 comments
Closed

Typescript Types & Other Clean Up #568

Jarch09 opened this issue Mar 31, 2020 · 16 comments

Comments

@Jarch09
Copy link
Contributor

Jarch09 commented Mar 31, 2020

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.

@roggervalf
Copy link
Contributor

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.

@aronhelser
Copy link
Collaborator

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.

@Jarch09
Copy link
Contributor Author

Jarch09 commented Mar 31, 2020

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?

@aronhelser
Copy link
Collaborator

aronhelser commented Mar 31, 2020 via email

Jarch09 added a commit to Jarch09/react-tooltip that referenced this issue Apr 4, 2020
@vicky-holcomb
Copy link

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

@Jarch09
Copy link
Contributor Author

Jarch09 commented Apr 8, 2020

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

@roggervalf
Copy link
Contributor

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

@skyqrose
Copy link

skyqrose commented Apr 8, 2020

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?

@aronhelser
Copy link
Collaborator

aronhelser commented Apr 8, 2020 via email

@Jarch09
Copy link
Contributor Author

Jarch09 commented Apr 8, 2020

@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
Copy link
Contributor Author

Jarch09 commented Apr 8, 2020

@TJANGEL - let's catch up offline

@skyqrose
Copy link

#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
Copy link

Works as of 4.2.4. Thanks!
PR to remove it from DefinitelyTyped: DefinitelyTyped/DefinitelyTyped#44079

@rathpc
Copy link

rathpc commented Apr 24, 2020

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.

@cgat
Copy link

cgat commented Dec 1, 2021

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

@danielbarion
Copy link
Member

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

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

8 participants