0

So I was reading StackOverflow questions and I found this one helpful, on different ways to change the state.

Now if I do something like this, I don't get my alert on state change. So would like to know if this is a bad approach ?? It was mentioned in the above question (most upvoted answer) that we can do something like this.

    export default function Test() {
      const [firstList, setFirstList] = useState([{ a: "2" }, { b: "3" }]);
      const [secondList, setSecondList] = useState([{ c: "4" }, { d: "5" }]);
    
      useEffect(() => {
        alert(JSON.stringify(firstList) + "        list is merged");
      },[firstList]);        

      function mergeList() {
        let previous = firstList;
        secondList.forEach((d) => previous.push(d));
        setFirstList(previous);
      }

  return <button onClick={mergeList}>Click</button>;
}

I can anyhow write my function this way and get my alert.

  function mergeList() {
    secondList.forEach((d) => setFirstList((firstList) => [...firstList, d]));
  }

So would like to know if the first approach is badly written?

Ranu Vijay
  • 793
  • 8
  • 15
  • 3
    You are directly mutating the component's state. As the question you linked says, you should never do that. *It was mentioned in the above question (most upvoted answer) that we can do something like this.* No, that question *does not* say that you're free to mutate state. – CertainPerformance Apr 18 '22 at 17:38
  • see this part of the most upvoted answer - When you need to change 'something' in the existing state, first get a copy of that 'something' from the current state. const currentStateCopy = [...this.state.a] Now, mutating currentStateCopy won't mutate the original state. Do operations over currentStateCopy and set it as the new state using setState(). this.state = {a: [1,2,3,4,5]}, currentStateCopy.push(6).this.setState({ a: currentStateCopy}) – Ranu Vijay Apr 18 '22 at 17:42
  • @CertainPerformance, do let me know if I miss interpreted this from the above answer. – Ranu Vijay Apr 18 '22 at 17:43
  • You did not make a copy. You are directly modifying the component's state. – CertainPerformance Apr 18 '22 at 17:44
  • ok, my bad, **let previous = firstList;** is not the copy. – Ranu Vijay Apr 18 '22 at 17:45
  • @RanuVijay: If you want to make a copy of the array, probably the simplest approach would be: `let previous = [...firstList];` – David Apr 18 '22 at 17:45
  • yes @David, I missed a point here so. got it now. – Ranu Vijay Apr 18 '22 at 17:46
  • Life is hard, it takes -2 points to understand things, while you learn. – Ranu Vijay Apr 18 '22 at 17:54

1 Answers1

1

1. You are mutating firstList state value, you should not do.

      function mergeList() {
        let previous = firstList;
        secondList.forEach((d) => previous.push(d));
        setFirstList(previous);
      }

I recommend to use the following.

      function mergeList() {
        const previous = [...firstList];
        secondList.forEach((d) => previous.push(d));
        setFirstList(previous);
      }

But please note that spread ... operator do shallow-copy for an array of object. In practice, if you need to change a field in object in an array, you may get problem because of the shallow copy. In that case, you should use deep-copy.

What is shallow copy and deep copy in JavaScript?

2. Your code is wrong and it will raise an error about reaching maximum deps rendering in case that secondList has many items.

Because you called state update function in forEach loop.

Never use it.

  function mergeList() {
    secondList.forEach((d) => setFirstList((firstList) => [...firstList, d]));
  }
Liki Crus
  • 1,585
  • 3
  • 10
  • 22
  • Yes, secondList can be anything so I was finding a way to do it in a more proper way. Missed the point to make a copy of the data and directly referenced the firstList. – Ranu Vijay Apr 18 '22 at 17:57
  • Please use the `setState` function like the first one on my answer. Your approach was almost correct. – Liki Crus Apr 18 '22 at 17:59
  • CAN you suggest document links to read more about shallow and deep copies? – Ranu Vijay Apr 18 '22 at 17:59
  • @RanuVijay [What is shallow copy and deep copy in JavaScript ?](https://www.geeksforgeeks.org/what-is-shallow-copy-and-deep-copy-in-javascript/) – Liki Crus Apr 18 '22 at 18:00
  • A doubt please, suppose I have 3,4, or any number of objects in my second list and i use **secondList.forEach((d) => setFirstList((firstList) => [...firstList, d]));**, why is my alert firing only once ? shouldn't it fire as many times as state change or the number of objects secondlist have.? – Ranu Vijay Apr 18 '22 at 20:56
  • @RanuVijay React updates state variable asynchronously. So similar updates could be done in batch mode. – Liki Crus Apr 18 '22 at 21:13
  • @RanuVijay don't use update code like you wrote – Liki Crus Apr 18 '22 at 21:14
  • I was just curious to check the number of alerts I get based on the number of objects. I myself was looking for a better approach so I would use the solution provided here. Thanks :) :) – Ranu Vijay Apr 18 '22 at 21:17
  • @RanuVijay if you wanna see multiple alerts, you can add a delay in your loop. comments and my answer are the correct ones. So no worries!!! – Liki Crus Apr 18 '22 at 21:21