Suriname0
Suriname0

Reputation: 571

Avoiding stale state in double useEffect?

I have two useEffect hooks in a component (InternalComponent) that displays a single data item. One useEffect tracks a count as state, POSTing to a database when the user increments the count. The second useEffect resets the count when the item tracked by the InternalComponent changes (due to external manipulation).

The problem is: the useEffect that updates the database will fire when the item changes; it will see the new item value, but will incorrectly send the database the count from the previous item. This occurs because the two useEffects fire "simultaneously", with the state that would indicate the count shouldn't be POSTed not being set until after the POST useEffect is run.

const InternalComponent = ({ item }) => {
  const [count, setCount] = useState(item.count);
  const [countChanged, setCountChanged] = useState(false);

  useEffect(() => {
    console.log(
      `Item is now ${item.id}; setting count from item and marking unchanged.`
    );
    setCount(item.count);
    setCountChanged(false);
  }, [item]);

  useEffect(() => {
    if (countChanged) {
      console.log(
        `Count changed for item ${item.id}, POSTing (id=${item.id}, count=${count}) to database.`
      );
    } else {
      console.log(
        `Count hasn't changed yet, so don't update the DB (id=${item.id}, count=${count})`
      );
    }
  }, [item, count, countChanged]);

  const handleButtonClick = () => {
    setCount(count + 1);
    setCountChanged(true);
  };

  return (
    <div>
      I'm showing item {item.id}, which has count {count}.<br />
      <button onClick={handleButtonClick}>Increment item count</button>
    </div>
  );
};

Minimal working example on Code Sandbox: https://codesandbox.io/s/post-with-stale-data-vfzh4j?file=/src/App.js

The annotated output:

1 (After button click) Count changed for item 1, POSTing (id=1, count=6) to database. 
2 (After item changed) Item is now 2; setting count from item and marking unchanged. 
3 Count changed for item 2, POSTing (id=2, count=6) to database. 
4 (useEffect runs twice) Count hasn't changed yet, so don't update the DB (id=2, count=50) 

Line 3 is the unwanted behavior: the database will receive a POST with the wrong item ID and that ideally shouldn't have been sent at all.

This feels like a simple/common problem: what design am I supposed to use to prevent the POST useEffect from firing with stale state? All the solutions I can easily think of seem absurd to me. (e.g. creating one InternalComponent for each item and only displaying one of them, combining all the useEffects into a single giant useEffect that tracks every state in the component, etc.) I'm sure I'm overlooking something obvious: any ideas? Thank you!

Upvotes: 3

Views: 834

Answers (3)

P.L.
P.L.

Reputation: 1488

Giving <InternalComponent /> a unique key, so it unmount and mount fresh again is definitely the way to go for a simple component, as mentioned by @drew-reese

Also I am proponent of making posting to database or APIs a deliberate action if I can get away with it. Making them a explicit action inside useCallback (where you add the count, AND, post to database) is a good option here for a simple update scenario.

As for why you are getting unwanted 'POSTing' to database when you switch item in your sample code, it is just because of your own bug, that you did not reset the countChange to false in your code once you had 'POST' it.

    //...
    useEffect(() => {
      if (countChanged) {
        //ADD THIS
        setCountChanged(false);

        console.log(
          `Count changed for item ${item.id}, POSTing (id=${item.id}, count=${count}) to database.`
        );
      } else {
        console.log(
          `Count hasn't changed yet, so don't update the DB (id=${item.id}, count=${count})`
        );
      }
    }, [item, count, countChanged]);
    //...

Upvotes: 1

Drew Reese
Drew Reese

Reputation: 203198

The issue is caused by using item as a dependency for both useEffect hooks. When the item prop updates the second useEffect hook is triggered with the old count state, then again a second time after the first useEffect hook updates the count state.

You basically just want to "reset" the InternalComponent state when the item prop updates. Just use the item's id as a React key on the InternalComponent.

Example:

<InternalComponent key={item.id} item={item} />

Then the key changes React throws away (unmounts) the previous instance and mounts a new instance with new state initialized as you expect.

Because InternalComponent is remounted/reset, the second useEffect hook is completely extraneous now and can be removed. The state values will pick up the new item prop value.

const InternalComponent = ({ item }) => {
  const [count, setCount] = useState(item.count);
  const [countChanged, setCountChanged] = useState(false);

  useEffect(() => {
    if (countChanged) {
      console.log(
        `Count changed for item ${item.id}, POSTing (id=${item.id}, count=${count}) to database.`
      );
    } else {
      console.log(
        `Count hasn't changed yet, so don't update the DB (id=${item.id}, count=${count})`
      );
    }
  }, [item, count, countChanged]);

  const handleButtonClick = () => {
    setCount(count + 1);
    setCountChanged(true);
  };

  return (
    <div>
      I'm showing item {item.id}, which has count {count}.<br />
      <button onClick={handleButtonClick}>Increment item count</button>
    </div>
  );
};

Edit avoiding-stale-state-in-double-useeffect

Upvotes: 2

Enfield Li
Enfield Li

Reputation: 2530

Well, you are not updating the items with it's own setter function, instead put it in a count state, which makes the logic all over the place, pretty hard to read, to be honest, I am here to propose a different solution that might solve your problem:

const InternalComponent = ({ items, setItems, index }) => {
  useEffect(() => {
    console.log("item count changed: ", items[index].count);
  }, [items, index]);

  const handleButtonClick = () => {
    setItems(
      items.map((item) =>
        item.id - 1 === index ? { ...item, count: item.count + 1 } : { ...item }
      )
    );
  };

  return (
    <div>
      I'm showing item {items[index].id}, which has count {items[index].count}.
      <br />
      <button onClick={handleButtonClick}>Increment item count</button>
    </div>
  );
};

This approach keeps the updated state, if you want to default back to it's original count value after clicking next item, it should be easy to do that. Let me know if this way solves the problem

Sandbox

Upvotes: 1

Related Questions