Do we need React.memo in all of the components?

[hamiidrahmani]

Hello,
As I reviewed and read this repo, It raised a question for me.
I'm not sure about this, but I would like to know your reasons for using React.memo in almost all components.
As I know about React.memo, it has a shallow comparison and it is mostly used in those kinds of components that have expensive calculations. Here you used that in a simple component like Portal. Maybe you thought so the children of Portal component has an expensive calculation, But I should say that the components that are passed to this component as a child can be memorized. I think this is not a proper use of React.memo. It's semantically problematic and arguably costs you more in terms of memory allocation and performance.
Now, as far as I knew, and based on the explanations I gave, I wanted to know your point of view on this issue, and I would be very happy to hear your opinion. Thank you.

[mimshins]

Hello,
As I reviewed and read this repo, It raised a question for me.
I'm not sure about this, but I would like to know your reasons for using React.memo in almost all components.
As I know about React.memo, it has a shallow comparison and it is mostly used in those kinds of components that have expensive calculations. Here you used that in a simple component like Portal. Maybe you thought so the children of Portal component has an expensive calculation, But I should say that the components that are passed to this component as a child can be memorized. I think this is not a proper use of React.memo. It's semantically problematic and arguably costs you more in terms of memory allocation and performance.
Now, as far as I knew, and based on the explanations I gave, I wanted to know your point of view on this issue, and I would be very happy to hear your opinion. Thank you.

Hello there,
First of all thanks for your time and contribution.

Generally I have my own guideline and sort of a checklist for "When to use React.memo".
Here's the list of the considerations I took before I use React.memo:

1. If it is a Pure Functional Component:
   Means the component is functional and given the same props, always renders the same output.
2. If it renders often
3. If it re-renders with the same props:
   Means the component is usually provided with the same props during re-rendering.
4. If it is medium to large size:
   It doesn't necessarily mean that it contains heavy loads of calculations (because you can always memoize those kind of calculations in the component itself), instead it contains a decent amount of UI Elements to reason props equality check.

However, before I wrote this replay I did some further investigations on the matter and reviewed our components. I did figure it out that using React.memo isn't necessarily a good idea when you're about to develop an UI Library, since the end-user may not follow the best practices to keep the component memoized.

Let me explain it through a few examples:
Let's say we have a memoized component that accepts a callback as a prop:

const Button = React.memo(({ onClick }) => <button onClick={onClick}>...</button>);

const App = () => {
  return <Button onClick={(e) => console.log(e)} />
};

Every time a parent component defines a callback for its child, it creates new function instances and since a new function instance has a different reference, the memoization of <Button /> component will breaks. The only workaround is that the parent component memoize onClick callback and pass it to the <Button />.

The same things happen when the child component accepts an object or even the children prop.

I know for a fact that comparing the tree returned by the component most of the time is more expensive than comparing a pair of props through an equality check; However, if user breaks the memoization then you gain no performance benefits but also run for naught the comparison function.

So to wrap this up, I'm going to remove React.memo from the components except for the components often re-render with the same props and also they don't accept the children prop, and I will leave the memoization to the end-user to do it on their own responsibilities.

import Button from "@sonnat/ui/Button";

const MemoizedButton = React.memo(Button) as typeof Button;

const App = () => {
  const handleOnClick = React.useCallback((event: MouseEvent) => {
    console.log(event);
  }, []);
  
  return <MemoizedButton onClick={handleOnClick} />;
};

Thanks again for your review, it helped a lot and we are going to have a better performance (Both speed-wise and memory-wise) because of this discussion.

[hamiidrahmani]

Hello again,
First of all, thanks for your time and consideration and also for the great explanations you provided with the example.
I'm very happy that I was able to help in this regard. I would be happy if I could work with you to resolve this issue.

[mimshins]

Well sure, that would be great.
You can PR this issue and if you needed to discuss further, feel free to contact me via [email protected].

And as a conclusion for this issue, I guess it's a better approach to remove all of the React.memos.
I don't think we have any components that needs it at this moment.

The last thing, I'm going to convert this discussion into an issue. Please make sure to accept the collaboration invitation so I can make you the assignee.

Thanks again.

[hamiidrahmani]

Thanks again. I will make a PR as soon as possible and remove all React.memo from the UI components.