facebook / react

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

Feedback for 'exhaustive-deps' lint rule #20097

Closed jiangyh1024 closed 3 years ago

jiangyh1024 commented 4 years ago

I'm using 'use-immer' package to replace useState

but this will throw 'exhaustive-deps' lint rule if using setState method inside hook like useeffect/usecallback

eg:

const [activeTab, setActiveTab] = useImmer(0);

React.useEffect(() => { setActiveTab(() => 1); }. []); // will require setActiveTab as a dependency

I know it can be solved by disabling the rule, but I don't think it is a good solution

kachkaev commented 4 years ago

Could you elaborate on the inconvenience you experience by keeping setActiveTab as a dependency? At our team, we also use third-party hooks and the setters (and other functions) are added to deps. This does not feel wrong or time-consuming.

In theory, there is no guarantee that a new version of useImmer will not start changing the reference to setActiveTab at some point. So if the rules of hooks are not followed, the code will break.

jiangyh1024 commented 4 years ago

using react.useState will not be required to add setState method as a dependency, so I thought there might be solutions to avoid this problem. seems this rule is defined in eslint-react-hook-deps

image

jordyvandomselaar commented 3 years ago

Is there a downside to adding the setActiveTab to your dependencies? Because it's a custom hook, React doesn't know if setActiveTab changes or not, thus requiring it in the dependency array.

It's the same when you pass dispatch from useReducer as a prop to a different component; even though it never changes, you have to pass it because at that moment React cannot know if it changes or not,

gaearon commented 3 years ago

The solution is to pass it. There is no problem doing it if it's a stable reference.

Jayatubi commented 11 months ago

The solution is to pass it. There is no problem doing it if it's a stable reference.

It would be better if there could be a user config to add exclude terms for the rule.