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

useSelector returns new value on each call even equalityFn returns true #1654

Closed
otakustay opened this issue Oct 29, 2020 · 7 comments
Closed

Comments

@otakustay
Copy link
Contributor

What is the current behavior?

  1. Open https://codesandbox.io/s/serene-franklin-pjkhu?file=/src/App.js
  2. Press button
  3. Inspect console log

We have some confusing logs in console like:

array equals: true 
selector value changed 

This indicates the return value of useSelector changes even its equalityFn returns true.

What is the expected behavior?

In DisplayValueList component, the value mustChange can change on every press, but the next useSelector should return a constant value because its selector function returns a static array and its equalityFn is set to an array equal function, the console log should be clean.

When the first useSelector is removed, the console output is clean as expected, this is even more confusing to me.

@markerikson
Copy link
Contributor

This is specifically because you're passing in a new selector function reference on each render pass. We always re-run the selector if the selector is a new reference, because your code could have changed what it's selecting entirely (ie, going from state => state.todos to state => state.posts).

If you were to extract the selector function definition outside the component, or memoize creating it, you'll see the same array result being returned.

@otakustay
Copy link
Contributor Author

Sorry but I still think equalityFn should work even selector is a new reference. When equalityFn is omitted and selector returns a reference equal object, useSelector doesn't trigger a re-render.

In many cases it's hard to keep selector reference equal, mostly when we select state from props:

const post = useSelector(s => {
  const post = s.posts[props.id];
  const author = s.users[post.author];
  return {...post, author};
});

I believe when user passes a equalityFn, this function should take responsibility to identify state equality between different selectors.

@markerikson
Copy link
Contributor

Mmm... I can see this as a potential point, yeah. Not something I have time to change myself right now, but if you or someone else wants to put up a PR, we can look at it.

@markerikson markerikson reopened this Nov 4, 2020
otakustay added a commit to otakustay/react-redux that referenced this issue Nov 7, 2020
@nightpool
Copy link
Contributor

Hey @otakustay! Thank you for opening this issue—I just had the same confusing problem, where I assumed that the equalityFn would be applied in both cases! This was what my code looked like:

   const items = useSelector(state => todoIds.map(id => state.entities.todos[id]), shallowEquals);
   useMemo(() => doSomethingExpensiveWith(items), [items]);

and I found to my surprise that the doSomethingExpensive was being called every time the component rendered! I had assumed that the shallowEquals, which I know is already memoizing the returned value when the store changes, would also be memoizing the returned value when the selector changed.

@otakustay Let me know if you need any help with that patch—would love to get this over the finish line!

@otakustay
Copy link
Contributor Author

@nightpool I have created #1660 days ago, waiting for a review or merge

@jgibson02
Copy link

jgibson02 commented Dec 3, 2020

I ran into this as well where we are using a selector for constructing translation string keys, but due to some race conditions they can get malformed during transitions.

We still want to reuse the selector logic so I was hoping to just override the equalityFn with () => true to freeze it to its initial value, and was confused that that didn't work.

It seems like @otakustay's PR should fix that since that's exactly what they've done in tests.

markerikson pushed a commit that referenced this issue Mar 22, 2021
Co-authored-by: nightpool <nightpool@users.noreply.github.com>
Co-authored-by: Tim Dorr <timdorr@users.noreply.github.com>
@markerikson
Copy link
Contributor

Released in https://github.com/reduxjs/react-redux/releases/tag/v7.2.3 .

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

4 participants