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

react-redux/useSelector-prefer-selectors returns false positive/negative when passing additional params #76

Open
mladenoff opened this issue Aug 25, 2021 · 1 comment

Comments

@mladenoff
Copy link

mladenoff commented Aug 25, 2021

If we have a selector like export const selectUser = (state, id) => state.users[id] that requires additional parameters passed to it from the component (in this case, id), this rule gives us useSelector should use a named selector function.eslint(react-redux/useSelector-prefer-selectors) when used in the component like const user = useSelector((state) => selectLead(state, id)).

It is worth noting that this is the syntax recommended by the Redux docs (when a memoized selector is not needed).
If we write a currying selector like export const selectUser = (id) => (state) => state.users[id] and use it in the component like const user = useSelector(selectUser(id)) the rule passes.

I don't think this current behavior follows the letter or the spirit of the rule as written. The return value of selectUser(id) is very much not a named function and we are creating a new function just as much as we are with the non-curried selector wrapped in the arrow function. I think the false positive here is because of

      if (selector && (
        selector.type === 'ArrowFunctionExpression' ||
        selector.type === 'FunctionExpression')
      ) {
        reportNoSelector(context, node);

in lib/rules/useSelector-prefer-selectors.js

Given all this it seems like the current rule is more accurately named react-redux/useSelector-prefer-named-selectors as alluded to by @DianaSuvorova in #54 (comment) on #54. It looks like the description was changed but the rule name was not changed, I guess to follow the pattern from react-redux/mapStateToProps-prefer-selectors?

With the current name it seems like the rules wants some additional options, but I am not entirely sure on how to implement them. I guess one to permit an anonymous function passed to useSelector when it contains a named function (can we even check for this? And maybe another to forbid/allow the curried named function as "selector creator"?

@DianaSuvorova
Copy link
Owner

@mladenoff thanks for the detailed explanation of the issue. Sounds like the name of the rule is miss-leading, if you can create a PR to change the name, I would be happy to review/approve it 😊
Sounds like renaming the rule should be the most straightforward way to avoid the confusion, let me know if you have other recommendations instead.

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

No branches or pull requests

2 participants