React: memo equality check function overrides state

Created on 28 Feb 2019  Â·  3Comments  Â·  Source: facebook/react

Do you want to request a feature or report a bug?
BUG

What is the current behavior?
When a component is wrapped with React.memo and is provided 2nd argument - equality check function, it overrides state in the parent component

What is the expected behavior?
It should not override the state in parent component

Reproduction and detailed explanation of the bug is within this codesandbox:
https://codesandbox.io/s/4j6ownx8x0

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Codesandbox

Note: I spent few hours verifying that this is a genuine bug and not a due to my misunderstanding, but I apologize if it is the latter.

Most helpful comment

Custom equality checks must include all props you care about. Including callbacks. (This is why generally you shouldn't supply your own comparison function — it's easy to make a mistake!)

The mistake you're making is that you're ignoring changes to props.onSelect. So the onSelect handler of both buttons "sees" the initial selectedStartDate state only (null). It's never updated. Therefore, if (!selectedStartDate) is always true and you always end up calling setSelectedStartDate.

If I fix your equality function, it works:

const equalityCheck = (prevProps, nextProps) =>
-  prevProps.comparedProp === nextProps.comparedProp;
+  prevProps.comparedProp === nextProps.comparedProp &&
+  prevProps.onSelect === nextProps.onSelect;

https://codesandbox.io/s/62961yk72z

You can also useCallback if you don't want to break memoization by always passing different functions. But in that case you need to make sure you don't close over anything that's not specified in deps. So you can't close over selectedStartDate in your check.

The most idiomatic fix to this is useReducer. It makes it easy to preserve callback identity while having more complicated state logic. Here's an example: https://codesandbox.io/s/62961yk72z.

All 3 comments

https://codesandbox.io/s/943916znlp
see what memo component renders here after each call

Custom equality checks must include all props you care about. Including callbacks. (This is why generally you shouldn't supply your own comparison function — it's easy to make a mistake!)

The mistake you're making is that you're ignoring changes to props.onSelect. So the onSelect handler of both buttons "sees" the initial selectedStartDate state only (null). It's never updated. Therefore, if (!selectedStartDate) is always true and you always end up calling setSelectedStartDate.

If I fix your equality function, it works:

const equalityCheck = (prevProps, nextProps) =>
-  prevProps.comparedProp === nextProps.comparedProp;
+  prevProps.comparedProp === nextProps.comparedProp &&
+  prevProps.onSelect === nextProps.onSelect;

https://codesandbox.io/s/62961yk72z

You can also useCallback if you don't want to break memoization by always passing different functions. But in that case you need to make sure you don't close over anything that's not specified in deps. So you can't close over selectedStartDate in your check.

The most idiomatic fix to this is useReducer. It makes it easy to preserve callback identity while having more complicated state logic. Here's an example: https://codesandbox.io/s/62961yk72z.

Many thanks for your elaborate explanation! I understand it now, hopefully others who make this same mistake will find their answers here as well

and thanks @iskandar17 for chiming in

Was this page helpful?
0 / 5 - 0 ratings

Related issues

zpao picture zpao  Â·  3Comments

framerate picture framerate  Â·  3Comments

MoOx picture MoOx  Â·  3Comments

UnbearableBear picture UnbearableBear  Â·  3Comments

zpao picture zpao  Â·  3Comments