im working on a relatively large React codebase and I've seen that the previous developers used memo and usecallback liberally with the thought that by using these would improve performance. Obviously not, here is an example

export default function QuestionHelper({ text }: { text: string }) {
  const [show, setShow] = useState<boolean>(false);

  const open = useCallback(() => setShow(true), [setShow]);
  const close = useCallback(() => setShow(false), [setShow]);

  return (
    <span style={{ marginLeft: 4 }}>
      <Tooltip show={show} text={text}>
        <QuestionWrapper
          onClick={open}
          onm ouseEnter={open}
          onm ouseLeave={close}
        >
          <Question size={16} />
        </QuestionWrapper>
      </Tooltip>
    </span>
  );
}

The app does work right now. But i'm getting push back of not fixing it because the app works as intended. How can I make a case that we should remove all unecessary memo and usecallback in our code?

Is there an objective way of telling the benefits of removing these?

CodePudding user response：

So removing the one in your example actually COULD potentially change things. If you memoize it with useCallback, then the open function persists between renders, which means the function holds the same place in memory each render cycle. If you removed it and just had

const open = () => setShow(true);

Then every time the component renders, this function is re-created, and thus has a new place in memory.

What's the implication of that? Well, let's say hypothetically your QuestionWrapper component export is wrapped with React.memo, since unnecessary re-renders can cause performance issues for this particular component. Well when comparing functions with === how does Javascript compare objects (functions are technically objects in JS)? It does a "reference comparison", i.e. it compares the address in memory of the 2 inputs to see if it's the same object. In the current code, while memoized with useCallback, the reference won't change between re-renders. But if you remove that, then it's reference WILL change every re-render, and React.memo would no longer work for QuestionWrapper since it would think it's getting a new object every time, even though the actual function and its logic doesn't ever change.

Now, maybe in your app that wouldn't matter, maybe QuestionWrapper can re-render lots of times without performance issues, or maybe that component isn't memoized anyway so it doesn't matter, but the point is, in general: don't try to refactor/optimize things that aren't causing issues, unless there would be big implications down the line for not doing.

In an ideal world, you would assess this case and yeah, maybe removing some of the memoization would technically be the best choice. However in the real world, you inherit code bases that are full of crap that isn't ideal, but you just work with it for several reasons, such as:

• While you think it might not matter, as per the example I gave there's a chance it could break things or introduce bugs. Better safe than sorry
• You only have a fixed amount of hours in the day, so you should prioritize more impactful work
• The "improvement" you get from the refactor is so small as to be negligible, so why bother

and various other reasons. I understand that sub-optimal stuff can be a bit annoying and we'd like our code to be super sleek, well-structured and optimized, but unfortunately we have to deal with the reality that in large, real-world applications code is often full of stuff like this. Some of it may just be laziness or ignorance on other developers' part, or some of it may actually be that way for a good reason that might not be immediately evident.

You're always free to voice concern, but unless you're quite experienced and fully understand the implications of such refactoring, and have literally nothing better to do, then if everyone is saying to just leave it as is, it's best to do so. There's almost certainly better uses of your time, and if you do something like accidentally break an application or introduce bugs just so you could do tiny optimization or fix things that weren't even causing any issues, you'll make yourself quite unpopular.

Bsides, in my experience with React, bugs re. memoization can be a real pain in the ass to track down due to the bizarre behavior they can cause.