Skip to content

Add typings #1511

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 1 commit into from
Sep 12, 2019
Merged

Add typings #1511

merged 1 commit into from
Sep 12, 2019

Conversation

stramel
Copy link
Contributor

@stramel stramel commented Sep 12, 2019

This should give us a starting point for officially supporting types. Let me know if anything looks off.
Pulled from https://gist.github.com/ChristianMurphy/53ca2413776e9c6327919e368993c19a

Fixes #207

@tannerlinsley
Copy link
Collaborator

This looks fantastic and is a great start to the types. Our plans for v7 are to host the types on the Definitely Typed repo, and include them as a dependency in the React Table repo. Would you be able to redirect your PR to the Definitely Typed repo?

Once it is merged there, I will gladly add the dependency to it on this end.

@stramel
Copy link
Contributor Author

stramel commented Sep 12, 2019

@tannerlinsley Is there a reason you don't want to maintain the typings in this repo? It is much easier to manage here and keep in sync with the versions, especially when you are running 2 versions (like now with the v6 and v7-alpha). DT was originally just a workaround.

@tannerlinsley
Copy link
Collaborator

Lots of discussion happened here:

#1383 (comment)

@stramel
Copy link
Contributor Author

stramel commented Sep 12, 2019

I get why that could be better if you have someone who is properly maintaining it. However, it doesn't appear that it is being regularly maintained and is out of date anyways. DT can be a PITA to deal with, coming from experience dealing with other package definitions. The result is as it currently stands, out of date typings and tons of forks of a gist being passed around, copied and pasted into projects.

Personally, I don't see how that setup is better. I also can't find a clear definition of how to add an alpha support into DT which again is why we have gists being shared and forked.

@stramel stramel closed this Sep 12, 2019
@tannerlinsley
Copy link
Collaborator

Let's do this then. Since you're the first who has actually submitted code, I'm going to trust your judgement on this one. Let's merge these types and get them set up and just see how it goes! We can always move/change things around if we have to.

@tannerlinsley tannerlinsley reopened this Sep 12, 2019
@tannerlinsley tannerlinsley merged commit 670a927 into TanStack:master Sep 12, 2019
@tannerlinsley
Copy link
Collaborator

As with all types in a non-TS library, how should we go about keeping these up to date?

@stramel
Copy link
Contributor Author

stramel commented Sep 12, 2019

I would be happy to watch the repo and help maintain the types.
I appreciate you hearing my concerns and merging this. I definitely need to update these types though. Will put out a PR here shortly shoring up some stuff.

@stramel stramel deleted the ms/add-typings branch September 12, 2019 15:47
@tannerlinsley
Copy link
Collaborator

Awesome. I really appreciate your input. If it helps you feel better, every good interaction I have with TS brings me one step closer to adopting it fully. You have helped in that cause :)

@stramel
Copy link
Contributor Author

stramel commented Sep 12, 2019

That's great to hear! It does take a jump but it definitely feels nice once you do. If you can hold off on releasing those types, I will put out a PR to ensure they are up-to-date with the latest alpha.

@tannerlinsley
Copy link
Collaborator

Okay. Can do.

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.

typings in react-table?
2 participants