eslint-react icon indicating copy to clipboard operation
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

Open Rel1cx opened this issue 1 year ago • 5 comments

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

Rel1cx avatar Aug 30 '24 07:08 Rel1cx

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~~.

Rel1cx avatar Jun 02 '25 03:06 Rel1cx

Does the Help Wanted tag imply that this is waiting for a contribution, or that @Rel1cx you may eventually get to it?

reteps avatar Jul 07 '25 18:07 reteps

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 avatar Sep 27 '25 06:09 Rel1cx

@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:

Image

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

karlhorky avatar Oct 02 '25 17:10 karlhorky

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

karlhorky avatar Oct 05 '25 16:10 karlhorky