-
Notifications
You must be signed in to change notification settings - Fork 46
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
Heterogeneous selectors typings #67
Conversation
d46a861
to
30baeed
Compare
Hi @toomuchdesign! MaintainabilityThe typings are of course getting bigger with this PR, but it seems to be the only way to keep backward compatibility with the previous version. As mentioned in the original discussion in reselect, otherwise we break code that used type parameters. There are also use cases where the original typings are just easier to use, so looks like we have to keep both. The maintainability will of course suffer, but it seems the only way forward. Breaking changesIn general, I cannot see any breaking changes, given that we keep both typings (the way it is in this PR). But to play it safe, I would rather follow reselect example and bump the version. All in all, I'm glad that we keep up to date with reselect typings and look forward to try it in work! 👌 |
@xsburg hello, what you think about #65 PR (this is an attempt to generate types automatically)? This should improve maintainability. |
30baeed
to
af8daec
Compare
…ny number of uniform selectors (cherry picked from commit ef8d5a4)
af8daec
to
c89f348
Compare
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
feature
What is the current behaviour? (You can also link to an open issue here)
Heterogeneous selectors and any number of uniform selectors not implemented.
What is the new behaviour?
dependencies
property added in typing, supported heterogeneous selectors and any number of uniform selectors.We mainly replicate what happened in reselect typing here and here.
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
I think so, because the reselect for similar changes made it.
Other information:
see conversation in #60
Please check if the PR fulfills these requirements: