0

Reading questions about useState hook I was thinking about setState. I'm always using intentionally the setState with the callback option to access the previous state and have all the elements for example in an array.

So I have made a quick example for representation in a functional component:

const [elems, setElems] = useState([]);

const addElemsMutate = (count) => {
  const newArray = [...elems];
  for (let i = 0; i < count; i++) {
    newArray.push(i);
  }
  setElems(newArray);
};

const addElemsUsePreviousState = (count) => {
  for (let i = 0; i < count; i++) {
    setElems(previousElems => [...previousElems, i]);
  }
}

return <>
    <button onClick={() => addElemsMutate(10)}>addElemsMutate</button>
    <button onClick={() => addElemsUsePreviousState(10)}>addElemsUsePreviousState</button>
    {elems.map((e, index) => <span key={index}>{e}</span>)}
</>

Question

I understand the setState is asynchronous, enqueues changes and it should not be directly mutated as the documentation states.

The result form both of the actions look the same on the UI. So my question is about what I have in addElemsMutate function:

  1. Is that considered a bad practice?
  2. What is the downside if I'm using still that option if there is any?

Thanks!

norbitrial
  • 14,716
  • 7
  • 32
  • 59
  • 1
    ehm, your `addElemsMutate` does NOT mutate `elems`, it mutates a copy of the array. for the most common problem regarding READING the immutable `elems`, see [useState set method not reflecting change immediately](https://stackoverflow.com/q/54069253/1176601) – Aprillion Jan 02 '20 at 18:55
  • @Aprillion That's my silly mistake with that function's naming. Anyway thanks for the link! – norbitrial Jan 02 '20 at 19:22

2 Answers2

2

There's a lot going on here that may not be intentional:

  • As mentioned in a comment, addElemsMutate() doesn't actually mutate the state.
  • addElemsUsePreviousState() enqueues count state changes instead of the single one dispatched by addElemsMutate().

You can combine the callback form of useState() with useCallback() as a small performance optimization.

const addElemsCallback = useCallback(count => {
    setElems(elems => {
        const newElems = [...elems];
        for (let i = 0; i < count; i++) {
            newElems.push(i);
        }
        return newElems;
    });
}, [setElems]);

Since setElems never changes, neither does addElemsCallback, and it's the only thing you need to pass as a dependency.

David Harkness
  • 35,992
  • 10
  • 112
  • 134
  • `useCallback` is a performance optimization, but it will neither `ensure changes happen correctly` nor `avoid unnecessary DOM manipulation`: 1. DOM `click` event is attached to `document` by `react-dom`, no DOM manipulations needed for swapping synthetic event handlers.. 2. the ` – Aprillion Jan 03 '20 at 15:35
  • @Aprillion Thanks, I haven't had as much time to dig into the internals as I'd like. I've updated the answer to address your points. Do you bother with `useCallback()` often? – David Harkness Jan 04 '20 at 21:47
  • I almost never use `useCallback`, since functions that are needed inside `useEffect` can be defined inside it and I don't create many memoized components that take a function as a prop – Aprillion Jan 05 '20 at 11:20
  • 1
    also just noticed, your `setState` is undefined (based on question code), did you mean `setElems(elems => { ... return newElems; })`? – Aprillion Jan 05 '20 at 11:23
  • @Aprillion Yes, another good catch. Thanks for being my IDE! :) – David Harkness Jan 17 '20 at 03:47
0

React best practices for changing states is to copy the current state into a new variable, make the changes there, and then change the state (as you have done so in your example).

This is because, when you change the state directly, instead of deep copying what the previous state was, you run into issues with references, which can leave some hard to find bugs.

alex067
  • 3,159
  • 1
  • 11
  • 17