Skip to content

Add TypeScript typings (revisited) #815

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

Closed
wants to merge 8 commits into from

Conversation

pelotom
Copy link

@pelotom pelotom commented Oct 25, 2017

This is a continuation of #541 which seems to have stalled. Fixed the merge conflict and upgraded @types/react.

@timdorr
Copy link
Member

timdorr commented Oct 25, 2017

What is this based on? There are a ton of old commits here.

@pelotom
Copy link
Author

pelotom commented Oct 25, 2017

It's based on #541, so it has all the same commits as there. I'm not sure why there are so many, but as you can see the actual number of final changes is pretty small. I can rebase and squash if you want, but I was erring on the side of preserving history.

@aksharpatel47
Copy link

@timdorr Can you give feedback on what more needs to be done to get this PR merged?

@timdorr timdorr mentioned this pull request Dec 7, 2017
@timdorr
Copy link
Member

timdorr commented Dec 7, 2017

Got rid of all the junk from the previous branch. No idea how it got to that state, but it's fixed up now.

@pelotom
Copy link
Author

pelotom commented Dec 7, 2017

It looks like the DefinitelyTyped typings have undergone significant changes since these typings were developed... I'm unsure now what the relative merits are now of this PR relative to what's there.

Any thoughts from the authors of the DT typings?

/cc @tkqubo @thasner @kenzierocks @clayne11 @tansongyang @NicholasBoll @mDibyo @pdeva

@timdorr
Copy link
Member

timdorr commented Dec 7, 2017

Other than taking on ownership, I also doubt the benefits of keeping them here. It causes pain when TS moves forward and we don't. Releases are held to our schedule, which doesn't usually align with TS's schedule. I'd argue against adding this burden.

@abenhamdine
Copy link

abenhamdine commented Dec 7, 2017

Other than taking on ownership, I also doubt the benefits of keeping them here. It causes pain when TS moves forward and we don't. Releases are held to our schedule, which doesn't usually align with TS's schedule. I'd argue against adding this burden.

On the other hand, I think it's easier to keep typings up to date when they are in the same repo.
I realized that while contributing to node-postgres : contributors to the main module "forget" to update typings or to ping typings maintainers, and it's totally impossible to track DT activity given the numbers of contributions.

@timdorr
Copy link
Member

timdorr commented Dec 7, 2017

True. I have no connectivity to the TS world. But at the same time, that's an argument for not keeping them here. None of the maintainers here have any experience with TS. We're not able to update them here or on DT. Moving them will not help with update lag.

@aikoven
Copy link

aikoven commented Dec 8, 2017

Agree with @timdorr. Including typings in the source project repo impedes updates on the typings, particularly it doesn't allow to make breaking changes in the typings without bumping the major version of the source project.

With Redux typings, we weren't able to use TS 2.0 features for a very long time because of this.

Luckily, Redux API is pretty simple in terms of typings. But the same cannot be said of React Redux, which involves higher-order functions — an area in which TS isn't perfect yet, but moves very fast to make it better. Which means that there could be a lot of changes to the typings.

@timdorr
Copy link
Member

timdorr commented Dec 8, 2017

OK, I'm going to close this out. Good DX is more important than us having iron-fisted control.

@timdorr timdorr closed this Dec 8, 2017
@thasner
Copy link

thasner commented Dec 8, 2017

The best scenario is to write code in TS. Then typings are generated automatically 😄.

Otherwise, assuming react-redux doesn't have an interest in TS, it's likely best to keep typings in DT where the community can make changes independently.

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.

7 participants