Skip to content

proposal: no-self-updating-effect#346

Open
NisargIO wants to merge 1 commit into
mainfrom
discovery/propose-no-self-updating-effect
Open

proposal: no-self-updating-effect#346
NisargIO wants to merge 1 commit into
mainfrom
discovery/propose-no-self-updating-effect

Conversation

@NisargIO
Copy link
Copy Markdown
Member

@NisargIO NisargIO commented May 22, 2026

Proposal: react-doctor/no-self-updating-effect

Status: 🟡 Auto-discovered draft proposal. Not yet implemented. Maintainer review wanted before any code lands.

Category state-and-effects
Severity warn
Source clusters NEW::no-self-updating-effect
Independent draft proposals 1
Backing evidence units 1

Sources

Discovered by the react-doctor-evals discovery flywheel mining bug-fix evidence across React OSS repos. The pipeline below produced this proposal:

OSS repo → Vercel Sandbox miner → EvidenceUnit → RuleDrafter (LLM) → RuleDedupe → THIS PR

Backing evidence

Validation prompt

FP-aware guidance for the react-review agent when triaging this rule:

Confirm the effect directly calls a local useState setter and that the same state variable appears in the hook's dependency array. Ignore mount-only effects with [], setters nested inside timers/subscriptions/promise callbacks, and guarded updates that reach a fixed point. This rule is meant for feedback loops that exhaustive-deps does not catch because the dependency array is present.

Fix prompt

Actionable fix suggestion surfaced to the user when the rule fires:

Remove the feedback loop by deriving the value during render, moving the write into an event handler, or guarding the update so it only runs when the target value actually changes. For example:

useLayoutEffect(() => {
  if (count !== nextCount) {
    setCount(nextCount);
  }
}, [count, nextCount]);

If the value is purely derived, delete the state and compute it directly instead.

Positive fixture (SHOULD trigger)

import { useLayoutEffect, useState } from "react";

function Counter() {
  const [count, setCount] = useState(0);

  useLayoutEffect(() => {
    setCount((value) => value + 1);
  }, [count]);

  return null;
}

Negative fixture (should NOT trigger)

import { useLayoutEffect, useState } from "react";

function Counter() {
  const [count, setCount] = useState(0);

  useLayoutEffect(() => {
    setCount((value) => value + 1);
  }, []);

  return null;
}

Proposed AST detector

Would land at packages/oxlint-plugin-react-doctor/src/plugin/rules/state-and-effects/no-self-updating-effect.ts:

import { EFFECT_HOOK_NAMES } from "../../constants/react.js";
import { defineRule } from "../../utils/define-rule.js";
import { getCallbackStatements } from "../../utils/get-callback-statements.js";
import { getEffectCallback } from "../../utils/get-effect-callback.js";
import { isComponentAssignment } from "../../utils/is-component-assignment.js";
import { isHookCall } from "../../utils/is-hook-call.js";
import { isNodeOfType } from "../../utils/is-node-of-type.js";
import { isUppercaseName } from "../../utils/is-uppercase-name.js";
import type { EsTreeNode } from "../../utils/es-tree-node.js";
import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js";
import type { Rule } from "../../utils/rule.js";
import type { RuleContext } from "../../utils/rule-context.js";
import { collectUseStateBindings } from "./utils/collect-use-state-bindings.js";

const getDirectSetterCall = (
  statement: EsTreeNode,
  setterNames: ReadonlySet<string>,
): EsTreeNodeOfType<"CallExpression"> | null => {
  const expression = isNodeOfType(statement, "ExpressionStatement") ? statement.expression : statement;
  if (!isNodeOfType(expression, "CallExpression")) return null;
  if (!isNodeOfType(expression.callee, "Identifier")) return null;
  if (!setterNames.has(expression.callee.name)) return null;
  return expression;
};

const hasStateNameInDeps = (depsNode: EsTreeNode, stateName: string): boolean => {
  if (!isNodeOfType(depsNode, "ArrayExpression")) return false;
  return (depsNode.elements ?? []).some(
    (element) => isNodeOfType(element, "Identifier") && element.name === stateName,
  );
};

export const noSelfUpdatingEffect = defineRule<Rule>({
  id: "no-self-updating-effect",
  severity: "warn",
  recommendation:
    "Remove the feedback loop: derive the value during render, move the write into an event handler, or guard the update so it reaches a fixed point.",
  create: (context: RuleContext) => {
    const checkComponent = (componentBody: EsTreeNode | null | undefined): void => {
      if (!componentBody || !isNodeOfType(componentBody, "BlockStatement")) return;
      const useStateBindings = collectUseStateBindings(componentBody);
      if (useStateBindings.length === 0) return;

      const setterNameToStateName = new Map<string, string>();
      const setterNames = new Set<string>();
      for (const binding of useStateBindings) {
        setterNameToStateName.set(binding.setterName, binding.valueName);
        setterNames.add(binding.setterName);
      }

      for (const statement of componentBody.body ?? []) {
        if (!isNodeOfType(statement, "ExpressionStatement")) continue;
        const effectCall = statement.expression;
        if (!isNodeOfType(effectCall, "CallExpression")) continue;
        if (!isHookCall(effectCall, EFFECT_HOOK_NAMES)) continue;
        if ((effectCall.arguments?.length ?? 0) < 2) continue;
        const depsNode = effectCall.arguments[1];
        if (!isNodeOfType(depsNode, "ArrayExpression")) continue;

        const callback = getEffectCallback(effectCall);
        if (!callback) continue;
        const callbackStatements = getCallbackStatements(callback);
        if (callbackStatements.length === 0) continue;

        for (const callbackStatement of callbackStatements) {
          const setterCall = getDirectSetterCall(callbackStatement, setterNames);
          if (!setterCall || !isNodeOfType(setterCall.callee, "Identifier")) continue;
          const stateName = setterNameToStateName.get(setterCall.callee.name);
          if (!stateName) continue;
          if (!hasStateNameInDeps(depsNode, stateName)) continue;

          context.report({
            node: setterCall,
            message: `${setterCall.callee.name}() inside a React effect depends on \`${stateName}\` and feeds the same state back into the effect — this causes a render loop. Remove the self-dependency or guard the update so it settles.`,
          });
        }
      }
    };

    return {
      FunctionDeclaration(node: EsTreeNodeOfType<"FunctionDeclaration">) {
        if (!node.id?.name || !isUppercaseName(node.id.name)) return;
        checkComponent(node.body);
      },
      VariableDeclarator(node: EsTreeNodeOfType<"VariableDeclarator">) {
        if (!isComponentAssignment(node)) return;
        if (
          !isNodeOfType(node.init, "ArrowFunctionExpression") &&
          !isNodeOfType(node.init, "FunctionExpression")
        ) {
          return;
        }
        checkComponent(node.init.body);
      },
    };
  },
});

Generated by `rde discover` (see [millionco/react-doctor-evals#11](https://github.com/millionco/react-doctor-evals/pull/11) for the pipeline). Implementation, test fixtures, and rule registration are deliberately deferred — this PR exists for maintainer triage of the proposal only. Reject, edit-and-approve, or merge after wiring as you see fit.

Note

Low Risk
Low risk because this PR only adds a markdown proposal document and does not change any shipped rule logic or runtime behavior.

Overview
Adds a new draft proposal doc proposals/no-self-updating-effect.md for a react-doctor/no-self-updating-effect rule that would warn on React effects which call a local useState setter while also depending on the same state (render-loop feedback pattern).

The document includes discovery/backing evidence links, validation and fix guidance, example positive/negative fixtures, and a sketch of the intended AST detector; implementation and registration are explicitly deferred.

Reviewed by Cursor Bugbot for commit b28de92. Bugbot is set up for automated code reviews on this repo. Configure here.

@vercel
Copy link
Copy Markdown

vercel Bot commented May 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
react-doctor-website Ready Ready Preview, Comment May 22, 2026 8:58am

@reactreview
Copy link
Copy Markdown

reactreview Bot commented May 22, 2026

No new issues

Reviewed by reactreview for commit 6109c5f. Configure here.

@NisargIO NisargIO force-pushed the discovery/propose-no-self-updating-effect branch from 681d04b to b28de92 Compare May 22, 2026 08:49
@aidenybai aidenybai marked this pull request as ready for review May 22, 2026 08:51
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b28de92. Configure here.

Comment thread proposals/no-self-updating-effect.md Outdated
const setterCall = getDirectSetterCall(
callbackStatement,
setterNames
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misses sync nested setters

Medium Severity

The detector currently only inspects top-level statements within an effect callback. This means it won't catch useState setter calls nested inside control flow structures (like if, loop, or try blocks), even if the corresponding state is in the dependency array and would cause an infinite re-render loop.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b28de92. Configure here.

context.report({
node: setterCall,
message: `${setterCall.callee.name}() inside a React effect depends on \`${stateName}\` and feeds the same state back into the effect — this causes a render loop. Remove the self-dependency or guard the update so it settles.`,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignores fixed-point carve-out

Medium Severity

The detector incorrectly flags patterns that don't cause actual render loops, such as guarded updates that reach a fixed point (which the validation prompt says to ignore) or updates where React bails out due to state equality. This leads to false positives.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b28de92. Configure here.

if (!isNodeOfType(expression.callee, "Identifier")) return null;
if (!setterNames.has(expression.callee.name)) return null;
return expression;
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicates existing setter walk utility

Low Severity

The proposal introduces getDirectSetterCall to locate setter calls in effect callbacks, but the plugin already exposes walkInsideStatementBlocks for the same “synchronous effect body” question, used by no-effect-chain and effect-needs-cleanup.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b28de92. Configure here.

Auto-discovered draft proposal. No implementation yet — see file body for the proposed detector + prompts + evidence. See millionco/react-doctor-evals#11 for the discovery pipeline.
@NisargIO NisargIO force-pushed the discovery/propose-no-self-updating-effect branch from b28de92 to 6109c5f Compare May 22, 2026 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant