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

Help wanted with Typescript PRs #125

Closed
ellbee opened this issue May 3, 2016 · 12 comments
Closed

Help wanted with Typescript PRs #125

ellbee opened this issue May 3, 2016 · 12 comments

Comments

@ellbee
Copy link
Collaborator

ellbee commented May 3, 2016

There are a couple of outstanding PRs that I don't feel qualified to review (#123 and #115) relating to Typescript. It would be great to give someone commit access to the project who has Typescript experience and feels able to review and merge these PRs. Would anyone like to help out?

@threehams
Copy link
Collaborator

I can review. I'm fairly new to Typescript, but the library's small enough that it shouldn't be a problem.

@ellbee
Copy link
Collaborator Author

ellbee commented Jun 15, 2016

@threehams That would be awesome. Thank you.

@threehams
Copy link
Collaborator

I'm going to start contributing some work to the declaration files, so it might be good to have a second collaborator, as well.

@b-smets
Copy link

b-smets commented Jul 4, 2016

Not sure who is really to blame for this but please don't put breaking TS changes in dot releases.
87df18a added a mandatory TProps generic parameter between 2.5.1 and 2.5.2 which causes TS to fail to compile when using the old version. Not good at all!

@threehams
Copy link
Collaborator

The work involved for types in 3.0 is very little, but I'm really concerned about reducing the number of exported types from the definition file. At the moment, internal and external interfaces are mixed together, and it's possible (likely?) that some people are importing and using types meant to be internal in their projects.

I'm also starting to think that including type definitions in a non-Typescript repo may not be the best idea, as narrowing types over time - especially as new features come out in 2.1 and beyond - requires major version releases in Reselect itself, even though the library itself hasn't changed at all.

Immutable.js has a similar problem. The type definitions in the repo are backwards compatible with 1.6, even though improvements in 1.8 - 2.0 (especially the polymorphic this type) make for much better definitions.

Some opinions here from @ellbee or anyone in the community would be greatly appreciated.

@aikoven
Copy link
Contributor

aikoven commented Nov 1, 2016

@threehams I totally agree with you. It would be great if we could specify different definition file for each TS version in typingns field of package.json.

For the time being, we can still keep separate versions of definitions in this repo, but only put to package.json the one that doesn't break semver. Users can pick the version they need via Typings.

@aikoven
Copy link
Contributor

aikoven commented Nov 11, 2016

I've been maintaining custom typings for reselect for my projects and can contribute them.

Also consider that TS 2.1 will include Mapped types that would allow us to correctly type createStructuredSelector.

@ellbee
Copy link
Collaborator Author

ellbee commented Nov 13, 2016

@threehams I am unsure on best practices here, for Typescript and Flow, so I will follow the lead of someone who knows what they are doing.

@aikoven You have commit access, right? I would be happy for you to handle the Typescript stuff if you are up for it.

@aikoven
Copy link
Contributor

aikoven commented Nov 14, 2016

@ellbee Ok, I'm on it. Are there any API changes for 3.0?

@ellbee
Copy link
Collaborator Author

ellbee commented Nov 15, 2016

@aikoven Thank you!

There is a slight change here:

f63e49c

@alex3165
Copy link
Contributor

alex3165 commented Mar 9, 2017

I updated the type definition file as well #224, Is it necessary to keep this issue open ?

@ellbee
Copy link
Collaborator Author

ellbee commented Mar 9, 2017

No, let's close it down. Thanks to everyone who helped!

@ellbee ellbee closed this as completed Mar 9, 2017
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

5 participants