Using useCallback for this particular code?

Question

I have this piece of code

// This is onChange event
function nameChangeHandler(e, field = 'Name') {
        const currentName = e.target.value;

        if (currentName.length >= 10) {
            if (!errors.some((x) => x.field == field)) {
                setErrors((state) => [
                    ...state,
                    {
                        field,
                        errorMessage:
                            field +
                            ' should be between 3 and 10 characters long',
                    },
                ]);
            }
        } else if (currentName.length <= 3) {
            if (!errors.some((x) => x.field == field)) {
                setErrors((state) => [
                    ...state,
                    {
                        field,
                        errorMessage:
                            field +
                            ' should be between 3 and 10 characters long',
                    },
                ]);
            }
        } else {
            setErrors((state) => state.filter((x) => x.field !== field));
        }
    }

This part of the code is redundant

  if (!errors.some((x) => x.field == field)) {
                setErrors((state) => [
                    ...state,
                    {
                        field,
                        errorMessage:
                            field +
                            ' should be between 3 and 10 characters long',
                    },
                ]);
            }

My question is if place it inside an useCallback hook. Will it be a good practice or it's stupid move. Refactored code should be looking something like this:

const errorSetter = useCallback((field) => {
        if (!errors.some((x) => x.field == field)) {
            setErrors((state) => [
                ...state,
                {
                    field,
                    errorMessage:
                        field + ' should be between 3 and 10 characters long',
                },
            ]);
        }
    }, []);

function nameChangeHandler(e, field = 'Name') {
        const currentName = e.target.value;

        if (currentName.length >= 10) {
            errorSetter(field)
        } else if (currentName.length <= 3) {
            errorSetter(field)
        } else {
            setErrors((state) => state.filter((x) => x.field !== field));
        }
    }

I tried adding it in useCallback and it works fine. All though, I'm new to React and don't yet know the best practices.

Answer 1

If the component you're using nameChangeHandler on is memoized, there's an argument for memoizing nameChangeHandler (rather than errorSetter); details in my answer here. But you could make errorSetter a utility function that lives outside the component, like this:

function errorSetter(field, setErrors) {
    setErrors((errors) => {
        if (errors.some((x) => x.field == field)) {
            return errors;
        }
        return [
            ...errors,
            {
                field,
                errorMessage: field + " should be between 3 and 10 characters long",
            },
        ];
    });
}

const YourComponent = () => {
    const nameChangeHandler = useCallback(function nameChangeHandler(e, field = "Name") {
        const currentName = e.target.value;

        if (currentName.length >= 10) {
            errorSetter(field, setErrors);
        } else if (currentName.length <= 3) {
            errorSetter(field, setErrors);
        } else {
            setErrors((state) => state.filter((x) => x.field !== field));
        }
    }, []);

    // ...
};

Notice how the utility function doesn't rely on anything it closes over, it just uses the field and setErrors it's passed; and the only thing nameChangeHandler uses that it closes over is setErrors, which is guaranteed to be stable. So nameChangeHandler can be memoized via useCallback (or useMemo, or useRef, which are other ways of doing it).

By making nameChangeHandler stable, you avoid unnecessary re-renders of the component you pass it to — if that component is memoized.

---

If I use setErrors() the component will re-render...

Usually, yes, but not necessarily. Here's what the documentation for functional (callback) updates says (my emphasis), but keep reading, it's oversimplifying a bit:

If your update function returns the exact same value as the current state, the subsequent rerender will be skipped completely.

More accurately, the first time you set identical state, your component function will get called again, but unless something is different the render stops there. From that point forward, if you set identical state, a "fast bail-out" optimization means it doesn't even call your component function.

Here's an example of that:

const { useState } = React;

const lowerCaseA = "a".charCodeAt(0);

let renderCounter = 0;
const Example = () => {
    const [errors, setErrors] = useState([]);

    console.log(`Render #${++renderCounter}, errors = ${errors.join(", ")}`);
    const setSameValue = () => {
        // Call `setErrors`, but have it return the array that's already in state
        setErrors((errors) => errors);
    };
    const setDifferentValue = () => {
        // Call `setErrors`, have it create and return a new new array
        setErrors((errors) => [...errors, String.fromCharCode(lowerCaseA + errors.length)]);
    };
    return (
        <div>
            Errors: {errors.join(", ")}
            <div>
                <input type="button" value="Set Same Value" onClick={setSameValue} />
                <input type="button" value="Set Different Value" onClick={setDifferentValue} />
            </div>
        </div>
    );
};

const root = ReactDOM.createRoot(document.getElementById("root"));
root.render(<Example />);

<div id="root"></div>

<script src="https://cdnjs.cloudflare.com/ajax/libs/react/18.1.0/umd/react.development.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/18.1.0/umd/react-dom.development.js"></script>

In that, if you click Set Different Value, naturally the component re-renders, since we're returning a different array. (The fact the array's contents change isn't really relevant.)

In contrast, if you click Set Same Value, we're setting the same array that's already there. You'll notice that the first time, your component function is called to render (but child components aren't), and if you click it again, even your component function isn't called. (See the discussion in this closed issue for why there's a distinction between the first time and subsequent times.)

...I had problem with the code where if a word is longer than 10 characters for every character (after the 10th) it would add another object to the state which I didn't need because it was duplicate. That's why I put the if statement with some(). If I have memoized the nameChangeHandler would that problem have been solved...

You still need that check (it's in the code above), memoization doesn't change that. But in the normal case, it won't cause a re-render.

Answer 2

It can be simplified using or operation like:

function nameChangeHandler(e, field = 'Name') {
    const currentName = e.target.value;

    if (currentName.length >= 10 || currentName.length <= 3) {
        if (!errors.some((x) => x.field == field)) {
            setErrors((state) => [
                ...state,
                {
                    field,
                    errorMessage:
                        field +
                        ' should be between 3 and 10 characters long',
                },
            ]);
        }
    } else {
        setErrors((state) => state.filter((x) => x.field !== field));
    }
}

to reduce redundant code. I don't think useCallback is what you need here. The only usage of useCallback I can imagine here is to wrap the whole function in useCallback to avoid re-rendering the element you're passing it to.