eslint-react
eslint-react copied to clipboard
[feat] New rule `react-x/no-circular-(use)-effect` to detect circular `set` functions and deps patterns in `useEffect` like hooks
Describe the problem
import { useEffect, useState } from "react";
/**
* @component
* @description CircularEffect1 has a circular effect with a depth of 1
*/
export function CircularEffect1() {
const [items, setItems] = useState([0, 1, 2, 3, 4]);
useEffect(() => {
setItems(x => [...x].reverse());
}, [items]);
return null;
}
/**
* @component
* @description CircularEffect2 has a circular effect with a depth of 2
*/
export function CircularEffect2() {
const [items, setItems] = useState([0, 1, 2, 3, 4]);
const [limit, setLimit] = useState(false);
useEffect(() => {
setItems(x => [...x].reverse());
}, [limit]);
// ...Many other hooks between the two `useEffect` calls
useEffect(() => {
setLimit(x => !x);
}, [items]);
// ...
return null;
}
/**
* @component
* @description CircularEffect3 has a circular effect with a depth of 3
*/
export function CircularEffect3() {
const [items, setItems] = useState([0, 1, 2, 3, 4]);
const [limit, setLimit] = useState(false);
const [count, setCount] = useState(0);
useEffect(() => {
setItems(x => [...x].reverse());
}, [limit]);
useEffect(() => {
setCount(x => x + 1);
}, [items]);
useEffect(() => {
setLimit(x => !x);
}, [count]);
return null;
}
Describe the solution you'd like
Merge all useEffect like hooks within the same component or custom hook, then detect the calls to set and dispatch functions and the dependencies.
Alternatives considered
Using directed graph to collect and detect circular between useEffect like hooks within the same component or custom hook.
Additional context
https://github.com/Rel1cx/eslint-react/issues/745#issuecomment-2319783265 https://x.com/dan_abramov2/status/1825697422494019851 https://x.com/fraser_drops/status/1825953286786265424 https://x.com/fraser_drops/status/1582408358883397633
Sorry for the late reply to those who are following this rule. The development of this rule will be carried out ~~after the completion of react-x/no-unnecessary-use-effect~~.
Does the Help Wanted tag imply that this is waiting for a contribution, or that @Rel1cx you may eventually get to it?
Update: The RC version of eslint-plugin-react-hooks introduces a new rule from the React team, set-state-in-effect, which validates against calling setState synchronously within an effect (see React Docs)
In codebases that adhere to this new set-state-in-effect rule, the problem that the rule proposed in this issue aims to prevent no longer exists. Therefore, I believe it is necessary to re-evaluate the need for this rule.
@Rel1cx I agree in theory - if teams were to enable these strict rules.
However, enabling the strict React Compiler rules react-hooks/set-state-in-effect and react-hooks/set-state-in-render, also along with react-hooks/purity definitely seems like "hard mode" though - my guess is that this would be disabled by many teams.
After a few hours of enabling all published react-hooks/* rules and using it on our codebases, I ran into a few hard-to-handle problems:
- https://github.com/reactjs/react.dev/issues/7012#issuecomment-3360284491
- https://github.com/reactjs/react.dev/pull/8029
Some further discussion also including @gaearon in these issues:
- https://github.com/facebook/react/issues/33041
- https://github.com/reactjs/react.dev/issues/7012
I'm considering whether these very strict rules should be disabled for our projects (and also for our students, which have it much harder since they are just starting out).
The current react-x/no-circular-effect proposal (based on my original issue https://github.com/Rel1cx/eslint-react/issues/745) focuses on a smaller problem of the "effect loop" or "render cascades" mentioned in the Twitter links of the issue description:
This rule would be much more scoped: code statically analyzable as leading to a loop. This would allow more teams to enable it without adding a lot of eslint-disable directives.
So I have the feeling that having this rule which detects patterns which will definitely lead to "effect loops" would be still valuable.
cc @NickvanDyke from eslint-plugin-react-you-might-not-need-an-effect and https://github.com/Rel1cx/eslint-react/issues/1114
I also opened an issue about the strictness of react-hooks/set-state-in-effect over here:
- https://github.com/facebook/react/issues/34743
Also, the React Compiler team is aware of problems like this and is thinking about how to better handle false positives:
- https://github.com/facebook/react/issues/34045#issuecomment-3137784707