react icon indicating copy to clipboard operation
react copied to clipboard

Bug: ESLint rule doesn't catch mistake with non-exhaustive dependencies

Open arendjr opened this issue 1 year ago • 6 comments

React version: 18.2

Steps To Reproduce

  1. Use the useState() hook.
    • For example: const [count, setCount] = useState(0)
  2. Create a function that directly uses the setter.
    • For example: const inc: () => setCount((count) => count + 1)
  3. Witness how the function using the setter is not required to be listed as dependency, leading to soundness issues.
    • For example:
      const incAndRemember = useCallback(() => {
        inc();
        setLast({ last: inc }); // We might be setting a stale reference here,
      }, []);                   // because `inc` doesn't need to be specified here.
      
  4. Behavior in other hooks will start to diverge from what was intended if we ever rely on the identity of the reference we set.

Link to code example: https://github.com/arendjr/react-unstable-handler

The current behavior

When a function inside a component (only) uses a setter of a useState() hook, that function is determined to be "stable" (non-reactive) despite its identity being unstable. Because the ESLint rule assumes the function is stable, it doesn't require it to be listed in dependency arrays, which may lead to soundness issues with other hooks, such as witnessed in the linked repository.

It appears the current behavior is in violation of the documentation regarding reactive values (https://react.dev/learn/lifecycle-of-reactive-effects#all-variables-declared-in-the-component-body-are-reactive ), which says:

All values inside the component (including props, state, and variables in your component’s body) are reactive. Any reactive value can change on a re-render, so you need to include reactive values as Effect’s dependencies.

The expected behavior

Whether a function is considered stable or not should be strictly determined by its identity.

arendjr avatar Mar 11 '24 14:03 arendjr