Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Updater function of set state in promise.then clause of useEffect are called twice and replacing state before promise.then doesn't work as expected. #29829

Open
lanceree opened this issue Jun 10, 2024 · 4 comments

Comments

@lanceree
Copy link

I'm encountering an issue where the updater function of set state within promise.then of useEffect hook is being called twice. Additionally, attempting to replace the state before a promise resolves and then setting the state after the promise resolves does not work as expected. This behavior contradicts the behavior described in the React documentation on queueing a series of state updates.

React version: 18.2

Steps To Reproduce

Code
https://codesandbox.io/p/sandbox/sad-smoke-fzm9ww

const Context = createContext({})

const ChildWithCount = () => {
  const [num, setNum] = useState(1)
  const { l, setL } = useContext(Context)
  const [loading, setLoading] = useState(false)
  console.log("ChildWithCount re-renders", )

  useEffect(() => {
    console.log("useEffect")
    setLoading(true)
    setL([])
    new Promise((resolve) => {
      resolve()
      // setTimeout(() => resolve(), 100)
    }).then(() => {
      setL((prev) => {
        console.log("prev", prev)
        return [...prev, ...[1, 2, 3]]
      })
    }).finally(() => {
      setLoading(false)
    })
  }, [num])

  return (
    <div>
      <button
        onClick={() => {
          setNum((n) => n + 1)
        }}
      >
        Update
      </button>
      <ul>
        {l.map((_, index) => {
          return <li key={index}>{_}</li>
        })}
      </ul>
    </div>
  )
}

const App = () => {
  console.log("App rerenders")
  const [l, setL] = useState([])
  const contextValue = { l, setL }
  return (
    <Context.Provider value={contextValue}>
      <ChildWithCount />
    </Context.Provider>
  )
}

  1. Run the code

The current behavior

In strict mode, the first rendering lead to double data (6 items) in the List, even though I reset the list before calling the promise function, it didn't work. By clicking the Re-fetch button, the list being normal with 3 items. But I noticed that not matter if its strict mode, the list was replaced twice by checking console logs that the console.log in then clause are called twice and the list also changes twice with a quick flash of 6 items then showing the expected 3 items.

The expected behavior

The initial list should only show 3 items and clicking the Re-fetch button shouldn't make the element flickering.

@lanceree lanceree added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jun 10, 2024
@eps1lon
Copy link
Contributor

eps1lon commented Jun 10, 2024

In StrictMode, useEffect functions are executed twice. This catches race conditions e.g. when num changes but the first Promise takes longer than the second Promise. You can avoid this by returning a cleanup function that cancels the update like so:

useEffect(() => {
  let cancelled = false;
   new Promise((resolve) => {
      resolve()
      // setTimeout(() => resolve(), 100)
    }).then(() => {

	  if (!cancelled) {
        setL((prev) => {
          console.log("prev", prev)
          return [...prev, ...[1, 2, 3]]
        })
      }
    }).finally(() => {
      if (!cancelled) {
        setLoading(false)
      }
    })
  return () => {
    cancelled = true
  }
})

In React 19, you should initiate the Promise when you're updating `num` instead of setting it in `useEffect` to avoid waterfalls. And then unwrap the promise with `use` e.g.

```js
const [num, setNum] = useState()
const [promise, setPromise] = useState(initialPromise)
const value = use(promise)

function handle() {
  setNum(newNum)
  setPromise(newPromise)
}

@eps1lon eps1lon added Resolution: Expected Behavior and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Jun 10, 2024
@lanceree
Copy link
Author

Thanks for the quick response @eps1lon, but what I'm also confused about is that why the updater function has been executed twice even though the useEffect only executed once without using strict mode. I can see the log "useEffect" printed once, but the "prev" has been printed twice. However, this behavior is gone when I use a setTimeout with no wait time in the promise constructor.

@lanceree
Copy link
Author

In StrictMode, useEffect functions are executed twice. This catches race conditions e.g. when num changes but the first Promise takes longer than the second Promise. You can avoid this by returning a cleanup function that cancels the update like so:

useEffect(() => {
  let cancelled = false;
   new Promise((resolve) => {
      resolve()
      // setTimeout(() => resolve(), 100)
    }).then(() => {

	  if (!cancelled) {
        setL((prev) => {
          console.log("prev", prev)
          return [...prev, ...[1, 2, 3]]
        })
      }
    }).finally(() => {
      if (!cancelled) {
        setLoading(false)
      }
    })
  return () => {
    cancelled = true
  }
})

In React 19, you should initiate the Promise when you're updating `num` instead of setting it in `useEffect` to avoid waterfalls. And then unwrap the promise with `use` e.g.

```js
const [num, setNum] = useState()
const [promise, setPromise] = useState(initialPromise)
const value = use(promise)

function handle() {
  setNum(newNum)
  setPromise(newPromise)
}

And what you suggested is that in my implementation the setL([]) is executed twice before the 2 setL(prev => [...prev, ...[1, 2, 3]]), so that's why I'm seeing the initial rendering renders [1, 2, 3, 1, 2, 3] instead of [1, 2, 3]?

@eps1lon
Copy link
Contributor

eps1lon commented Jun 11, 2024

what I'm also confused about is that why the updater function has been executed twice even though the useEffect only executed once without using strict mode.

Updater functions may be invoked multiple times to rebase state updates (e.g. if something suspended). That's the production behavior StrictMode is preparing you for. Your state updater function needs to be resilient against being invoked multiple times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants