-
Notifications
You must be signed in to change notification settings - Fork 672
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
Move combiner function before selector functions for type safety #366
Comments
+1 the benefits of this would also include a better typing signature. const fooSelector = (state: RootState) => state.foo;
const barSelector = createSelector(fooSelector, foo => {
fooBar: foo.bar;
}); Rather than |
I reached the same conclusion to move the combiner as the first function but didn't say anything since I figured it would be too disruptive to the library. I'm all for this. This also could allow for some better features moving forward supposing the spread operators gets better and better support by TypeScript |
TS now supports mapping tuples, including arguments. This might be enough of a workaround to get strong typing on the current API, but it would still be much easier and simpler to fix the location of the combiner function or remove the other signatures. |
In JavaScript, spread parameters are only valid as the last parameter of a function to prevent ambiguity. Type checkers like TypeScript and Flow rely on this assumption as well for checking function argument types.
Unfortunately, even though TypeScript (as of version 3) and Flow support spread parameters, they cannot be used with Reselect's incorrect spread argument order. The combiner function is last, when it should be first so the selector functions can be a final spread parameter. This is necessary to take full advantage of all possible usages of Reselect besides the argument combinations hard coded in the TypeScript and Flow definitions, and also solves semantic confusion where Reselect has a different argument order for variable argument orders than most JavaScript APIs.
Alternatively, we could simply remove the API signatures that don't use the Array of selector functions. This would not break any code that is only using Arrays of selector functions.
The text was updated successfully, but these errors were encountered: