Using useEffect() properly is sometimes not that easy. Imagine we have the following simple app using the Counter component:

import { useState, useEffect } from 'react';

const Counter = ({ onOdd, onEven }) => {
    const [count, setCount] = useState(0);

    useEffect(
        () => {
            console.log('Inside useEffect()');

            if (count % 2 === 0) {
                onEven(count);
            } else {
                onOdd(count);
            }
        },
        [count, onOdd, onEven]
    );

    return (
        <button
            type="button"
            onClick={() => setCount(count => count   1)}
        >
            {count}
        </button>
    );
}

const App = () => {
    const [isDarkMode, setIsDarkMode] = useState(false);

    return (
        <div style={{
            backgroundColor: isDarkMode ? 'black' : 'white',
        }}>
            <Counter
                onOdd={count => console.log(`Odd count: ${count}`)}
                onEven={count => console.log(`Even count: ${count}`)}
            />
            <button
                type="button"
                onClick={() => setIsDarkMode(isDarkMode => !isDarkMode)}
            >
                Toggle dark mode
            </button>
        </div>
    );
}

export default App;

The app does two things:

1. It includes a Count button that increments its counter by 1. This components allows to inject two functions: onOdd and onEven. Whenever the counter changes, either onOdd or onEven is called, depending on the counter... being odd or even.
2. There is also a dark mode toggle. The only purpose I added it is to have something that causes the Counter to re-render for other reason than changing the count.

Now, the app works with one quirk - whenever we toggle the dark/light mode, the onOdd or onEven is being called. That's wrong, but understandable - we're creating new functions on each render, so useEffect() is being called.

I can think of 4 ways to fix this behavior:

1. Remove onOdd and onEven from useEffect() dependencies. It will fix the behavior, but it's considered a problem. The linter would complain about it, as we're losing data integrity. In theory, if we really change these callbacks, they should be re-run, right? That would be "the React way".
2. Move the callback functions outside of the App component:

const onOdd = count => console.log(`Odd count: ${count}`);
const onEven = count => console.log(`Even count: ${count}`);

const App = () => {
    // ...
    return (
        // ...
            <Counter
                onOdd={onOdd}
                onEven={onEven}
            />
        // ...
    );
}

This is a good and fast solution, but it's only possible because we don't use hooks or state inside these callbacks. What if we did?

3. Using useCallback() in App component:

const App = () => {
    // ...

    const onOdd = useCallback(
        count => console.log(`Odd count: ${count}`),
        []
    );

    const onEven = useCallback(
        count => console.log(`Even count: ${count}`),
        []
    );

    return (
        // ...
            <Counter
                onOdd={onOdd}
                onEven={onEven}
            />
        // ...
    );
}

4. Memoizing the callback functions in Counter component. If we had thousands of components using Counter component, it would still mean only one place to memoize these functions. I'm not sure if that makes sense though.

How do React gurus approach this problem? I wanted to keep the example as simple as possible, so option #2 will work perfectly and would probably be preferable. But what if we needed to keep these callbacks inside the App component?

Is it always the parent component responsible to memoize all callbacks it passes to the child? If so, is it a recognized pattern to always memoize all functions passed as props (and perhaps any other objects) with useCallback() or useMemo()?

CodePudding user response：

I'm not properly a React Guru, but I consider all first three approaches to have their sweet spot, the 4th does not make sense. The only one to be careful with is the first one, since removing functions from deps, might lead to stale state issues, so if you know what you are doing, you may suppress lint warn ( I do that sometimes and know many others do that as it has been discussed extensiveley here https://github.com/facebook/react/issues/14920 ), otherwise it's better you avoid this approach.
The point number 2 is preferred everytime you have pure functions, always try to place your pure functions out of React components, inside some other folder like utils, misc, etc...
As per point number 3 that's the preferred way to handle functions declared inside React components, always memoize them with *useCallback* ( or useMemo if you need to perform calculations before to return a function ) , and there's nothing bad with doing that in the parent component. If you find yourself having dozens or hundreds of them and fear code pollution, consider that custom hooks let you to organize your code smartly, you could make a custom hook like useMemoizedHandlers inside your App component, where you create and memoize all your handlers and use it like:

const { 
        handler1,
        handler2,
        handler3
    } = useMemoizedHandlers()

CodePudding user response：

Options 2 and 3 are both absolutely valid and common, used interchangeably depending on whether the function has render cycle dependencies. Option 1 is a big no no. Option 4 is not really memoization at all - you can create stable references from functions passed as props but you cannot memoize the functions themselves as they've already been created anew.

Is it always the parent component responsible to memoize all callbacks it passes to the child?

In an application context I would say yes as this is the only way to enable React.memo on the consuming component's props. However, libraries will often convert functions to stable refs in the child, in case users forget to memoize themselves (or just as improved DX). Again, this is not the same as memoization, but it does mean that you can avoid the dependency issues highlighted in your question.

Is it a recognized pattern to always memoize all functions passed as props (and perhaps any other objects) with useCallback() or useMemo()?

You will find both memoization maxis and minimalists in the React community so it's hard to say that there's an accepted standard. Generally, you can get away with not doing it until you need it, like your example. However, purely from personal experience, once you do it a few times out of necessity it starts to become a habit as it reduces the possibility that bugs like this can occur.