John Doener
John Doener

Reputation: 119

Why is useState within useEffect not working in React?

I use useEffect() to get a Firestore snapshot and parallel I want to count a value:

  const [counter, setCounter] = useState({ mona: 0, phil: 0 });
  useEffect(() => {
    onSnapshot(q, (snapshop) => {
      setTasks(
        snapshop.docs.map((doc) => {
          if (doc.data().wer === "Mona") {
            console.log("Mona + 1"); // This get's executed as expected (e.g. 3 times)
            setCounter({ ...counter, mona: counter.mona + 1 });
          }
          if (doc.data().wer === "Phil") {
            console.log("Phil + 1"); // This get's executed as expected (e.g. 6 times)
            setCounter({ ...counter, phil: counter.phil + 1 });
          }
          return {
            ...doc.data(),
            id: doc.id,
            timestamp: doc.data().timestamp?.toDate().getTime(),
          };
        })
      );
      setLoading(false);
    });
  }, []);

  useEffect(() => {
    console.log({ counter }); //this get's executed only 2 times.
  }, [counter]);

When the console.log() within the map() get executed correct, why does the setCounter doesn't execute or update the counter correct?

The console.log({ counter }); btw gives nothing more than:

{counter: {mona: 0, phil: 0}}
{counter: {mona: 0, phil: 1}}

Upvotes: 0

Views: 2747

Answers (2)

GACy20
GACy20

Reputation: 979

React sometimes batches updates to the state. Which means all your call to setCounter only trigger one effect.

Moreover the value of counter inside your function is also updated at the end of the function, therefore you are losing updates.

What you should do:

  • First of all pass a callback to setCounter instead of using the value of counter. So change:

    setCounter({ mona: counter.mona, phil: counter.phil + 1 });
    

    to:

    setCounter(counter => ({ mona: counter.mona, phil: counter.phil + 1 }));
    
  • To force useEffect to be called multiple times you have to opt-out of batched updates using ReactDOM.flushSync:

    import { flushSync } from 'react-dom';
    
    // ...
    
    flushSync(() => setCounter(counter => ({ mona: counter.mona, phil: counter.phil + 1 })));
    

    In this way your useEffect should be called for every single change of the counter. Obviously this is less efficient than having the updates batched.


Since you are reloading the whole dataset everytime you want to re-count everything on each call to onSnapshot instead of simply modifying the current value.

In that case you can do this:

const newCounter = { mona: 0, phil: 0};
snapshop.docs.map((doc) => {
          if (doc.data().wer === "Mona") {
            console.log("Mona + 1"); // This get's executed as expected (e.g. 3 times)
            newCounter.mona += 1;
          }
          if (doc.data().wer === "Phil") {
            console.log("Phil + 1"); // This get's executed as expected (e.g. 6 times)
            newCounter.phil += 1;
          }
          // ...
});
setCounter(newCounter);

So you just compute the result and call setCounter once outside the loop with the final count. In this case you don't need to read the old state since you recompute it from scratch.

You could keep the old code and add a setCounter({mona: 0, phil: 0}) outside the loop, but I believe it would be less efficient than computing the values outside react hooks and only calling the setCounter once.

Upvotes: 1

Quentin
Quentin

Reputation: 944568

The function you pass to useEffect closes over the counter variable.

When you call setCounter it updates counter in the store, and the hook re-renders. The effect hook doesn't run again because none of the dependencies ([] - there are none) have changed.

Next time the event handler set up by onSnapshot is triggered, it uses the same value of counter as the previous time. This means that counter.phil is still 0 inside the effect hook. You add 1 to 0 again and call setCounter but this value is the same as the previous value.

Since counter hasn't changed this time, the second effect hook which depends on the value of counter doesn't get triggered.


Pass a function to setCounter to get the most recent value instead of the original closed over value:

setCounter((latestCounter) => { ...latestCounter, phil: latestCounter.phil + 1 });

Upvotes: 4

Related Questions