facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.3k stars 46.96k forks source link

Bug: Callback referencing to outdated state when assigned as part of initial state object #18247

Closed Huldoser closed 4 years ago

Huldoser commented 4 years ago

Callback referencing to outdated state when assigned as part of initial state object.

I am not actually sure if this is a bug or intended behavior, and if it is intended I would love to have a lint rule or react warning me about it.

React version: 16.13.0

Steps To Reproduce

import React from "react";

function App() {
  const increaseCount = () =>
    setCountObj({ ...countObj, count: countObj.count + 1 });

  const [countObj, setCountObj] = React.useState({
    count: 0,
    onButtonClick: increaseCount
  });

  return (
    <div>
      <h3>count: {countObj.count}</h3>
      <button onClick={countObj.onButtonClick}>Click to increase count</button>
    </div>
  );
}

export default App;

Link to code example: https://codesandbox.io/s/pensive-mendeleev-f25is

The current behavior

This is a simplified use case of an issue I encountered lately.

When clicking the button multiple times the count stays at 1, it seems that the callback inside the initial state referencing to outdated state value every time its called

The expected behavior

The state inside the callback should be undefined or referencing to the correct state.

Solution

I solved the issue by removing the callback from the state and creating 'actualState' variable that is a merge between the state with the callback.

bvaughn commented 4 years ago

This isn't a bug. It's a combination of how the useState API works (initializing only once) and how closures work in JavaScript.

I wouldn't suggest putting callbacks into your state- for reasons like this (and also because I think it's probably rarely, if ever, necessary).

If, for some reason, it is actually necessary, you could use the function form of updating state:

  const increaseCount = () =>
    setCountObj(prevState => ({ ...prevState, count: prevState.count + 1 }));

  const [countObj, setCountObj] = React.useState({
    count: 0,
    onButtonClick: increaseCount
  });
Huldoser commented 4 years ago

@bvaughn In this case I agree that it should be avoided to use callback inside the initial state but its not obvious.

Maybe we should add eslint warning as it leads to confusing and hard to debug behaviour?

bvaughn commented 4 years ago

Maybe! Feel free to submit a PR that adds a warning like that for discussion :smile: