react: Bug: useEffect closure not called when dependencies changed

I run into a very weird issue where the dependency of useEffect changes, but its closure is not called. The component renders correctly with the updated value due to a state change of a parent router, just the useEffect closure is not called.

React version: 17.0.0.2 and 18.0.0.rc1

Steps To Reproduce

Go to the reproducing CodeSandbox https://codesandbox.io/s/useeffect-bug-oz4k9k?file=/pages/index.tsx
Open the CodeSandbox Console
Click the “reload without parameters” button and then the “trigger bug” button
Observe the logged messages. We see 2 renders with {"status":"updated_value"}, but no useEffect closure call for that value.

Link to code example:

https://codesandbox.io/s/useeffect-bug-oz4k9k?file=/pages/index.tsx

https://github.com/fabb/react-useeffect-bug

Please note that it is a reduced test example from a much bigger project, so don‘t wonder about the weird usage of the ref.

The current behavior

In the reproducing example, the useEffect closure is not called for the updated value. Further changes correctly call the closure though.

The expected behavior

The useEffect closure should be called in the reproducing example. We see that the component renders with the updated value, so the useEffect dependencies definitely changed.

cc @gaearon as discussed on twitter

About this issue

Original URL
State: closed
Created 2 years ago
Reactions: 1
Comments: 17

Most upvoted comments

Thanks @valen214 for a compact repro.

There is no bug in React here. The issue is that this code is reading a mutable value during rendering. This makes the behavior unpredictable because the rendering result depends on the moment you read it. When it changes, React won’t know to re-render the component. And from React’s perspective, if the state has not changed, it’s safe to bail out of the update.

Concretely, code like this is wrong:

function MyComponent() {
  let something = window.something // Bad: reading mutable state!
  let something2 = someRef.current // Bad: also reading mutable state!
  useEffect(() => {
    // ...
  }, [something, something2])

If you need to read mutable data during rendering, you should either put it in state and update in useEffect (before 18) or use React.useSyncExternalStore (in 18+).

In the original case (Next.js example), the issue is similarly with storedStatusRef.current — which is reading from a mutable field during render. Render result should only depend on props/state/context. See https://beta.reactjs.org/apis/useref for more info on this.

Hope this helps!

gaearon on Apr 20, 2022

But the status DOES change after the first render when clicking the buttons, as can be observed in the console log.

fabb on Mar 30, 2022

I think you’re misunderstanding the example.

Note that the re-render IS triggered, by a state change of the router caused by the Router.replace. The ref also changes, but we do not rely on the ref to cause the re-render. Afaik the useEffect checks its dependency array on every render of the containing component, thus it should call the closure in this case. It does work in most cases, just not in this one. Since it‘s reliably reproducible, it would be really interesting what the underlying issue is.

The curious thing is that removing one of the setLoaded(true) calls „fixes“ the double re-render with the new value and the useEffect closure is called correctly. My wild guess is that there is optimization logic in react which should prevent unnecessary re-renders when primitive state values are set to the same value again (setLoaded(true) even though loaded is already true), and there is a bug in that optimization logic when refs are involved.

fabb on Mar 8, 2022