From 6e97b70bebdef427b179c7741e14200664b4bce5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 7 May 2026 11:25:30 +0000 Subject: [PATCH 1/7] feat(react-doctor): add no-effect-chain rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New rule (severity: warn) flagging the §7 anti-pattern from React's 'You Might Not Need an Effect' guide: useEffect(() => { if (card.gold) setGoldCount(c => c + 1); }, [card]); useEffect(() => { if (goldCount > 3) setRound(r => r + 1); }, [goldCount]); useEffect(() => { if (round > 5) setIsGameOver(true); }, [round]); Each link adds an extra render to the tree below the component. The chain is also rigid — setting `card` to a value from the past re-fires every downstream effect. Detector (per component body): 1. Collect every top-level useEffect call and, for each: - depNames: Identifier names in the dep array - writtenStateNames: state names whose setter is called in body - isExternalSync: body returns cleanup OR contains a recognized external-system call (subscribe / addEventListener / fetch / setInterval / new MutationObserver / etc.) OR mutates a ref 2. For every ordered pair (A, B) of distinct effects: edge iff (writes(A) ∩ deps(B)) ≠ ∅ AND ¬isExternalSync(A) AND ¬isExternalSync(B) 3. Report on every reader effect B with the chained state name. The article calls out one legitimate 'chain' — a multi-step network cascade where each effect re-fetches based on the previous step's result. Those effects all have isExternalSync=true (they contain fetch), so the rule won't fire. Complements the existing no-cascading-set-state rule (intra-effect multi-setter detector) without overlapping. Tests: 5 regression cases. flags: - article §7 Game-style three-effect chain does NOT flag: - single effect with multiple setters (covered by no-cascading-set-state) - article's GOOD network-cascade exception (each effect calls fetch) - chat-connection effect that shares deps with state-reset effect (createConnection().connect() / .disconnect() is external sync) - two effects whose written/read state sets are disjoint Co-authored-by: Aiden Bai --- packages/react-doctor/src/oxlint-config.ts | 1 + packages/react-doctor/src/plugin/constants.ts | 48 ++++ packages/react-doctor/src/plugin/index.ts | 2 + .../src/plugin/rules/state-and-effects.ts | 205 ++++++++++++++++++ packages/react-doctor/src/utils/run-oxlint.ts | 3 + .../fixtures/basic-react/src/state-issues.tsx | 25 +++ .../tests/regressions/state-rules.test.ts | 184 ++++++++++++++++ .../react-doctor/tests/run-oxlint.test.ts | 6 + 8 files changed, 474 insertions(+) diff --git a/packages/react-doctor/src/oxlint-config.ts b/packages/react-doctor/src/oxlint-config.ts index ce21d7ec..9712c94b 100644 --- a/packages/react-doctor/src/oxlint-config.ts +++ b/packages/react-doctor/src/oxlint-config.ts @@ -231,6 +231,7 @@ export const GLOBAL_REACT_DOCTOR_RULES: Record = { "react-doctor/no-derived-state-effect": "warn", "react-doctor/no-fetch-in-effect": "warn", "react-doctor/no-cascading-set-state": "warn", + "react-doctor/no-effect-chain": "warn", "react-doctor/no-effect-event-handler": "warn", "react-doctor/no-effect-event-in-deps": "error", "react-doctor/no-prop-callback-in-effect": "warn", diff --git a/packages/react-doctor/src/plugin/constants.ts b/packages/react-doctor/src/plugin/constants.ts index eef0d063..59494692 100644 --- a/packages/react-doctor/src/plugin/constants.ts +++ b/packages/react-doctor/src/plugin/constants.ts @@ -346,6 +346,54 @@ export const MUTATING_ROUTE_SEGMENTS = new Set([ export const EFFECT_HOOK_NAMES = new Set(["useEffect", "useLayoutEffect"]); export const HOOKS_WITH_DEPS = new Set(["useEffect", "useLayoutEffect", "useMemo", "useCallback"]); + +// Used by `no-effect-chain` to decide whether an effect is doing +// "real" external-system synchronization (in which case effects on +// either side of the chain are exempt, per the article's own caveat +// about cascading network fetches) versus pure internal reactivity +// (which is the anti-pattern). A cleanup return is the strongest +// signal; the curated method list covers the rest. +export const EXTERNAL_SYNC_MEMBER_METHOD_NAMES = new Set([ + // Subscriptions / event listeners + "subscribe", + "addEventListener", + "addListener", + "on", + "watch", + "listen", + "sub", + // Imperative widget lifecycle (createConnection().connect()/.disconnect()) + "connect", + "disconnect", + "open", + "close", + // Network + "fetch", + "post", + "put", + "patch", + "delete", +]); + +export const EXTERNAL_SYNC_DIRECT_CALLEE_NAMES = new Set([ + "fetch", + "ky", + "got", + "wretch", + "ofetch", + "setInterval", + "setTimeout", + "requestAnimationFrame", + "requestIdleCallback", + "queueMicrotask", +]); + +export const EXTERNAL_SYNC_OBSERVER_CONSTRUCTORS = new Set([ + "IntersectionObserver", + "MutationObserver", + "ResizeObserver", + "PerformanceObserver", +]); export const CHAINABLE_ITERATION_METHODS = new Set(["map", "filter", "forEach", "flatMap"]); export const STORAGE_OBJECTS = new Set(["localStorage", "sessionStorage"]); diff --git a/packages/react-doctor/src/plugin/index.ts b/packages/react-doctor/src/plugin/index.ts index 47075cb8..b5ccfc9f 100644 --- a/packages/react-doctor/src/plugin/index.ts +++ b/packages/react-doctor/src/plugin/index.ts @@ -180,6 +180,7 @@ import { noDerivedStateEffect, noDerivedUseState, noDirectStateMutation, + noEffectChain, noEffectEventHandler, noEffectEventInDeps, noFetchInEffect, @@ -201,6 +202,7 @@ const plugin: RulePlugin = { "no-derived-state-effect": noDerivedStateEffect, "no-fetch-in-effect": noFetchInEffect, "no-cascading-set-state": noCascadingSetState, + "no-effect-chain": noEffectChain, "no-effect-event-handler": noEffectEventHandler, "no-effect-event-in-deps": noEffectEventInDeps, "no-prop-callback-in-effect": noPropCallbackInEffect, diff --git a/packages/react-doctor/src/plugin/rules/state-and-effects.ts b/packages/react-doctor/src/plugin/rules/state-and-effects.ts index 1c4105ba..2914056f 100644 --- a/packages/react-doctor/src/plugin/rules/state-and-effects.ts +++ b/packages/react-doctor/src/plugin/rules/state-and-effects.ts @@ -1,6 +1,9 @@ import { CASCADING_SET_STATE_THRESHOLD, EFFECT_HOOK_NAMES, + EXTERNAL_SYNC_DIRECT_CALLEE_NAMES, + EXTERNAL_SYNC_MEMBER_METHOD_NAMES, + EXTERNAL_SYNC_OBSERVER_CONSTRUCTORS, HOOKS_WITH_DEPS, MUTATING_ARRAY_METHODS, RELATED_USE_STATE_THRESHOLD, @@ -1153,3 +1156,205 @@ export const noSetStateInRender: Rule = { }; }, }; + +// HACK: §7 of "You Might Not Need an Effect" — chains of computations: +// +// useEffect(() => { if (card.gold) setGoldCardCount(c => c + 1); }, [card]); +// useEffect(() => { if (goldCardCount > 3) setRound(r => r + 1); }, [goldCardCount]); +// useEffect(() => { if (round > 5) setIsGameOver(true); }, [round]); +// +// Each link adds one extra render to the tree below the component. +// More importantly, the chain is rigid: setting `card` to a value from +// the past re-fires every downstream effect. +// +// `noCascadingSetState` (already shipped) catches multi-setter calls +// inside ONE effect; it does NOT see across effects. This rule +// complements it by detecting the cross-effect dependence. +// +// Detector (per component body): +// 1. Collect every top-level useEffect call and, for each: +// - depNames: Identifier names in the dep array +// - writtenStateNames: state names whose setter is called in the body +// - isExternalSync: body returns cleanup OR contains a recognized +// external-system call (subscribe / addEventListener / fetch / +// setInterval / new MutationObserver / etc.) OR mutates a ref +// 2. For every ordered pair (A, B) of distinct effects: +// edge iff (writes(A) ∩ deps(B)) ≠ ∅ AND ¬isExternalSync(A) +// AND ¬isExternalSync(B) +// 3. Report on every effect B that is the target of any edge, +// naming the chained state and the upstream effect's writer. +// +// The article calls out one legitimate "chain" — a multi-step network +// cascade where each effect re-fetches based on the previous step's +// result. Those effects all have `isExternalSync = true` because they +// contain `fetch`, so the rule won't fire. +const findTopLevelEffectCalls = (componentBody: EsTreeNode): EsTreeNode[] => { + const effectCalls: EsTreeNode[] = []; + if (componentBody?.type !== "BlockStatement") return effectCalls; + for (const statement of componentBody.body ?? []) { + if (statement.type !== "ExpressionStatement") continue; + const expression = statement.expression; + if (expression?.type !== "CallExpression") continue; + if (!isHookCall(expression, EFFECT_HOOK_NAMES)) continue; + effectCalls.push(expression); + } + return effectCalls; +}; + +const collectDepIdentifierNames = (effectNode: EsTreeNode): Set => { + const depNames = new Set(); + const depsNode = effectNode.arguments?.[1]; + if (depsNode?.type !== "ArrayExpression") return depNames; + for (const element of depsNode.elements ?? []) { + if (element?.type === "Identifier") depNames.add(element.name); + } + return depNames; +}; + +const collectWrittenStateNamesInEffect = ( + effectCallback: EsTreeNode, + setterToStateName: Map, +): Set => { + const writtenStateNames = new Set(); + walkAst(effectCallback, (child: EsTreeNode) => { + if (child.type !== "CallExpression") return; + if (child.callee?.type !== "Identifier") return; + const stateName = setterToStateName.get(child.callee.name); + if (stateName) writtenStateNames.add(stateName); + }); + return writtenStateNames; +}; + +const isExternalSyncEffect = (effectCallback: EsTreeNode): boolean => { + if (effectCallback.body?.type === "BlockStatement") { + const statements = effectCallback.body.body ?? []; + for (const statement of statements) { + if (statement.type === "ReturnStatement" && statement.argument) return true; + } + } else if (effectCallback.body?.type !== "BlockStatement") { + // Concise arrow body — `useEffect(() => something())`. If the + // expression itself is an external sync call, the effect is + // single-statement-external; otherwise it can't be a chain link + // anyway because chains require setter calls in the body. + } + + let didFindExternalCall = false; + walkAst(effectCallback, (child: EsTreeNode) => { + if (didFindExternalCall) return false; + + if (child.type === "NewExpression") { + const constructor = child.callee; + if ( + constructor?.type === "Identifier" && + EXTERNAL_SYNC_OBSERVER_CONSTRUCTORS.has(constructor.name) + ) { + didFindExternalCall = true; + } + return; + } + + if (child.type === "AssignmentExpression") { + if ( + child.left?.type === "MemberExpression" && + child.left.property?.type === "Identifier" && + child.left.property.name === "current" + ) { + didFindExternalCall = true; + } + return; + } + + if (child.type !== "CallExpression") return; + + if ( + child.callee?.type === "Identifier" && + EXTERNAL_SYNC_DIRECT_CALLEE_NAMES.has(child.callee.name) + ) { + didFindExternalCall = true; + return; + } + + if ( + child.callee?.type === "MemberExpression" && + child.callee.property?.type === "Identifier" && + EXTERNAL_SYNC_MEMBER_METHOD_NAMES.has(child.callee.property.name) + ) { + didFindExternalCall = true; + } + }); + + return didFindExternalCall; +}; + +interface EffectInfo { + node: EsTreeNode; + depNames: Set; + writtenStateNames: Set; + isExternalSync: boolean; +} + +export const noEffectChain: Rule = { + create: (context: RuleContext) => { + const checkComponent = (componentBody: EsTreeNode | null | undefined): void => { + if (!componentBody || componentBody.type !== "BlockStatement") return; + + const useStateBindings = collectUseStateBindings(componentBody); + if (useStateBindings.length === 0) return; + const setterToStateName = new Map(); + for (const binding of useStateBindings) { + setterToStateName.set(binding.setterName, binding.valueName); + } + + const effectInfos: EffectInfo[] = []; + for (const effectCall of findTopLevelEffectCalls(componentBody)) { + const callback = getEffectCallback(effectCall); + if (!callback) continue; + effectInfos.push({ + node: effectCall, + depNames: collectDepIdentifierNames(effectCall), + writtenStateNames: collectWrittenStateNamesInEffect(callback, setterToStateName), + isExternalSync: isExternalSyncEffect(callback), + }); + } + if (effectInfos.length < 2) return; + + const reportedNodes = new Set(); + for (const writerEffect of effectInfos) { + if (writerEffect.isExternalSync) continue; + if (writerEffect.writtenStateNames.size === 0) continue; + for (const readerEffect of effectInfos) { + if (readerEffect === writerEffect) continue; + if (readerEffect.isExternalSync) continue; + if (readerEffect.depNames.size === 0) continue; + + let chainedStateName: string | null = null; + for (const writtenName of writerEffect.writtenStateNames) { + if (readerEffect.depNames.has(writtenName)) { + chainedStateName = writtenName; + break; + } + } + if (!chainedStateName) continue; + if (reportedNodes.has(readerEffect.node)) continue; + reportedNodes.add(readerEffect.node); + + context.report({ + node: readerEffect.node, + message: `useEffect reacts to "${chainedStateName}" which is set by another useEffect — chains of effects add an extra render per link and become rigid as code evolves. Compute what you can during render and write all related state inside the event handler that originally fires the chain`, + }); + } + } + }; + + return { + FunctionDeclaration(node: EsTreeNode) { + if (!node.id?.name || !isUppercaseName(node.id.name)) return; + checkComponent(node.body); + }, + VariableDeclarator(node: EsTreeNode) { + if (!isComponentAssignment(node)) return; + checkComponent(node.init?.body); + }, + }; + }, +}; diff --git a/packages/react-doctor/src/utils/run-oxlint.ts b/packages/react-doctor/src/utils/run-oxlint.ts index 21b24fa9..a1891d74 100644 --- a/packages/react-doctor/src/utils/run-oxlint.ts +++ b/packages/react-doctor/src/utils/run-oxlint.ts @@ -46,6 +46,7 @@ const RULE_CATEGORY_MAP: Record = { "react-doctor/no-derived-state-effect": "State & Effects", "react-doctor/no-fetch-in-effect": "State & Effects", "react-doctor/no-cascading-set-state": "State & Effects", + "react-doctor/no-effect-chain": "State & Effects", "react-doctor/no-effect-event-handler": "State & Effects", "react-doctor/no-effect-event-in-deps": "State & Effects", "react-doctor/no-prop-callback-in-effect": "State & Effects", @@ -240,6 +241,8 @@ const RULE_HELP_MAP: Record = { "Use `useQuery()` from @tanstack/react-query, `useSWR()`, or fetch in a Server Component instead", "no-cascading-set-state": "Combine into useReducer: `const [state, dispatch] = useReducer(reducer, initialState)`", + "no-effect-chain": + "Compute as much as possible during render (e.g. `const isGameOver = round > 5`) and write all related state inside the event handler that originally fires the chain. Each effect link adds an extra render and makes the code rigid as requirements evolve", "no-effect-event-handler": "Move the conditional logic into onClick, onChange, or onSubmit handlers directly", "no-derived-useState": diff --git a/packages/react-doctor/tests/fixtures/basic-react/src/state-issues.tsx b/packages/react-doctor/tests/fixtures/basic-react/src/state-issues.tsx index fdcdf69a..bc4dd039 100644 --- a/packages/react-doctor/tests/fixtures/basic-react/src/state-issues.tsx +++ b/packages/react-doctor/tests/fixtures/basic-react/src/state-issues.tsx @@ -135,6 +135,30 @@ const ConditionalSetStateInRenderComponent = ({ count }: { count: number }) => { return

{prevCount}

; }; +interface Card { + gold: boolean; +} + +const EffectChainComponent = ({ card }: { card: Card | null }) => { + const [goldCount, setGoldCount] = useState(0); + const [round, setRound] = useState(1); + useEffect(() => { + if (card !== null && card.gold) { + setGoldCount((c) => c + 1); + } + }, [card]); + useEffect(() => { + if (goldCount > 3) { + setRound((r) => r + 1); + } + }, [goldCount]); + return ( +
+ {goldCount} {round} +
+ ); +}; + const UncontrolledInputComponent = () => { // HACK: explicit `` keeps TypeScript happy while the // RUNTIME initializer stays undefined — that's what trips the @@ -169,5 +193,6 @@ export { DirectStateMutationComponent, SetStateInRenderComponent, ConditionalSetStateInRenderComponent, + EffectChainComponent, UncontrolledInputComponent, }; diff --git a/packages/react-doctor/tests/regressions/state-rules.test.ts b/packages/react-doctor/tests/regressions/state-rules.test.ts index c772f824..5145b139 100644 --- a/packages/react-doctor/tests/regressions/state-rules.test.ts +++ b/packages/react-doctor/tests/regressions/state-rules.test.ts @@ -219,6 +219,190 @@ export const Loader = () => { }); }); +describe("no-effect-chain", () => { + it("flags the article §7 Game-style cross-effect chain", async () => { + // https://react.dev/learn/you-might-not-need-an-effect#chains-of-computations + const projectDir = setupReactProject(tempRoot, "no-effect-chain-game", { + files: { + "src/Game.tsx": `import { useEffect, useState } from "react"; + +interface Card { gold: boolean } + +export const Game = ({ card }: { card: Card | null }) => { + const [goldCount, setGoldCount] = useState(0); + const [round, setRound] = useState(1); + const [isGameOver, setIsGameOver] = useState(false); + + useEffect(() => { + if (card !== null && card.gold) { + setGoldCount((c) => c + 1); + } + }, [card]); + + useEffect(() => { + if (goldCount > 3) { + setRound((r) => r + 1); + setGoldCount(0); + } + }, [goldCount]); + + useEffect(() => { + if (round > 5) { + setIsGameOver(true); + } + }, [round]); + + return
{isGameOver ? "over" : round}
; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-effect-chain"); + // The downstream effects (reading goldCount and round) should each be + // flagged once. The first effect (writing goldCount) doesn't read state + // set elsewhere, so it isn't flagged. + expect(hits.length).toBeGreaterThanOrEqual(2); + expect(hits.some((hit) => hit.message.includes("goldCount"))).toBe(true); + expect(hits.some((hit) => hit.message.includes("round"))).toBe(true); + }); + + it("does NOT flag a single effect with multiple setters (covered by no-cascading-set-state)", async () => { + const projectDir = setupReactProject(tempRoot, "no-effect-chain-single-effect", { + files: { + "src/Settings.tsx": `import { useEffect, useState } from "react"; + +export const Settings = () => { + const [name, setName] = useState(""); + const [email, setEmail] = useState(""); + useEffect(() => { + setName("default"); + setEmail("default@example.com"); + }, []); + return
{name} {email}
; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-effect-chain"); + expect(hits).toHaveLength(0); + }); + + it("does NOT flag the article's GOOD network-cascade exception", async () => { + // The article explicitly notes that a chain of effects is appropriate + // when each effect synchronizes with the network. Each fetch-bearing + // effect is `isExternalSync = true` and thus exempt. + const projectDir = setupReactProject(tempRoot, "no-effect-chain-network", { + files: { + "src/ShippingForm.tsx": `import { useEffect, useState } from "react"; + +export const ShippingForm = ({ country }: { country: string }) => { + const [cities, setCities] = useState(null); + const [city, setCity] = useState(null); + const [areas, setAreas] = useState(null); + + useEffect(() => { + let ignore = false; + fetch(\`/api/cities?country=\${country}\`) + .then((response) => response.json()) + .then((json) => { + if (!ignore) setCities(json); + }); + return () => { + ignore = true; + }; + }, [country]); + + useEffect(() => { + if (city === null) return; + let ignore = false; + fetch(\`/api/areas?city=\${city}\`) + .then((response) => response.json()) + .then((json) => { + if (!ignore) setAreas(json); + }); + return () => { + ignore = true; + }; + }, [city]); + + return ( + + ); +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-effect-chain"); + expect(hits).toHaveLength(0); + }); + + it("does NOT flag a chat-connection effect even if it shares deps with another effect", async () => { + // External-system synchronization (createConnection().connect()) sets + // the upstream effect's `isExternalSync = true`, which exempts both + // sides of the would-be edge. + const projectDir = setupReactProject(tempRoot, "no-effect-chain-chat", { + files: { + "src/Chat.tsx": `import { useEffect, useState } from "react"; + +declare const createConnection: (url: string) => { + connect: () => void; + disconnect: () => void; +}; + +export const Chat = ({ roomId }: { roomId: string }) => { + const [messages, setMessages] = useState([]); + + useEffect(() => { + const connection = createConnection(roomId); + connection.connect(); + return () => connection.disconnect(); + }, [roomId]); + + useEffect(() => { + setMessages([]); + }, [roomId]); + + return
    {messages.map((line) =>
  • {line}
    • )}
; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-effect-chain"); + expect(hits).toHaveLength(0); + }); + + it("does NOT flag two effects whose written/read state sets are disjoint", async () => { + const projectDir = setupReactProject(tempRoot, "no-effect-chain-disjoint", { + files: { + "src/Profile.tsx": `import { useEffect, useState } from "react"; + +export const Profile = ({ userId, theme }: { userId: string; theme: string }) => { + const [name, setName] = useState(""); + const [highlight, setHighlight] = useState(""); + useEffect(() => { + setName(userId.toUpperCase()); + }, [userId]); + useEffect(() => { + setHighlight(theme === "dark" ? "white" : "black"); + }, [theme]); + return {name}; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-effect-chain"); + expect(hits).toHaveLength(0); + }); +}); + describe("no-uncontrolled-input", () => { it("flags `value` without onChange / readOnly", async () => { const projectDir = setupReactProject(tempRoot, "no-uncontrolled-input-no-onchange", { diff --git a/packages/react-doctor/tests/run-oxlint.test.ts b/packages/react-doctor/tests/run-oxlint.test.ts index 0cd2dcdc..066c6320 100644 --- a/packages/react-doctor/tests/run-oxlint.test.ts +++ b/packages/react-doctor/tests/run-oxlint.test.ts @@ -128,6 +128,12 @@ describe("runOxlint", () => { ruleSource: "rules/state-and-effects.ts", severity: "warning", }, + "no-effect-chain": { + fixture: "state-issues.tsx", + ruleSource: "rules/state-and-effects.ts", + severity: "warning", + category: "State & Effects", + }, "no-effect-event-handler": { fixture: "state-issues.tsx", ruleSource: "rules/state-and-effects.ts", From e0701fc7eba42a6185816a61cf676d41b7843ea1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 7 May 2026 11:26:12 +0000 Subject: [PATCH 2/7] chore: add changeset for no-effect-chain Co-authored-by: Aiden Bai --- .changeset/no-effect-chain-rule.md | 55 ++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 .changeset/no-effect-chain-rule.md diff --git a/.changeset/no-effect-chain-rule.md b/.changeset/no-effect-chain-rule.md new file mode 100644 index 00000000..d7e08666 --- /dev/null +++ b/.changeset/no-effect-chain-rule.md @@ -0,0 +1,55 @@ +--- +"react-doctor": minor +--- + +feat(react-doctor): add `no-effect-chain` rule + +New rule (severity: `warn`) flagging the §7 anti-pattern from React's +[You Might Not Need an Effect](https://react.dev/learn/you-might-not-need-an-effect#chains-of-computations) +guide: + +```tsx +useEffect(() => { if (card?.gold) setGoldCount(c => c + 1); }, [card]); +useEffect(() => { if (goldCount > 3) setRound(r => r + 1); }, [goldCount]); +useEffect(() => { if (round > 5) setIsGameOver(true); }, [round]); +``` + +Each link adds an extra render to the tree below the component. The +chain is also rigid — setting `card` to a value from the past +re-fires every downstream effect. + +The fix the article recommends is to compute as much as possible +during render (`const isGameOver = round > 5`) and write all related +state inside the event handler that originally fires the chain. + +## Detector + +For every component body: + +1. Collect every top-level `useEffect` call and extract: + - `depNames`: identifier names in the dep array + - `writtenStateNames`: state names whose setter is called inside + - `isExternalSync`: the effect returns a cleanup function OR + contains a recognized external-system call (`subscribe`, + `addEventListener`, `fetch`, `setInterval`, `new MutationObserver`, + etc.) OR mutates a ref (`ref.current = …`) +2. For every ordered pair `(A, B)` of distinct effects, draw an edge + iff `writes(A) ∩ deps(B) ≠ ∅` **and** neither `A` nor `B` is + `isExternalSync`. +3. Report on every reader effect that is the target of any edge, + naming the chained state. + +## Complements `no-cascading-set-state` + +`no-cascading-set-state` (already shipped) catches multi-setter calls +inside **one** effect. `no-effect-chain` catches chains **across** +effects. They detect orthogonal shapes and can fire independently. + +## Article's GOOD exception + +The article explicitly notes that a chain of effects is appropriate +when each effect synchronizes with the network (e.g. cascading +dropdowns where each fetches options for the next). Each fetch-bearing +effect has `isExternalSync = true` and is exempt — verified by a +regression test mirroring the `ShippingForm` example from +*Removing Effect Dependencies*. From 163c78d5853d10eb02079a95846fcc845ec964ea Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 7 May 2026 11:47:48 +0000 Subject: [PATCH 3/7] chore: drop changeset Co-authored-by: Aiden Bai --- .changeset/no-effect-chain-rule.md | 55 ------------------------------ 1 file changed, 55 deletions(-) delete mode 100644 .changeset/no-effect-chain-rule.md diff --git a/.changeset/no-effect-chain-rule.md b/.changeset/no-effect-chain-rule.md deleted file mode 100644 index d7e08666..00000000 --- a/.changeset/no-effect-chain-rule.md +++ /dev/null @@ -1,55 +0,0 @@ ---- -"react-doctor": minor ---- - -feat(react-doctor): add `no-effect-chain` rule - -New rule (severity: `warn`) flagging the §7 anti-pattern from React's -[You Might Not Need an Effect](https://react.dev/learn/you-might-not-need-an-effect#chains-of-computations) -guide: - -```tsx -useEffect(() => { if (card?.gold) setGoldCount(c => c + 1); }, [card]); -useEffect(() => { if (goldCount > 3) setRound(r => r + 1); }, [goldCount]); -useEffect(() => { if (round > 5) setIsGameOver(true); }, [round]); -``` - -Each link adds an extra render to the tree below the component. The -chain is also rigid — setting `card` to a value from the past -re-fires every downstream effect. - -The fix the article recommends is to compute as much as possible -during render (`const isGameOver = round > 5`) and write all related -state inside the event handler that originally fires the chain. - -## Detector - -For every component body: - -1. Collect every top-level `useEffect` call and extract: - - `depNames`: identifier names in the dep array - - `writtenStateNames`: state names whose setter is called inside - - `isExternalSync`: the effect returns a cleanup function OR - contains a recognized external-system call (`subscribe`, - `addEventListener`, `fetch`, `setInterval`, `new MutationObserver`, - etc.) OR mutates a ref (`ref.current = …`) -2. For every ordered pair `(A, B)` of distinct effects, draw an edge - iff `writes(A) ∩ deps(B) ≠ ∅` **and** neither `A` nor `B` is - `isExternalSync`. -3. Report on every reader effect that is the target of any edge, - naming the chained state. - -## Complements `no-cascading-set-state` - -`no-cascading-set-state` (already shipped) catches multi-setter calls -inside **one** effect. `no-effect-chain` catches chains **across** -effects. They detect orthogonal shapes and can fire independently. - -## Article's GOOD exception - -The article explicitly notes that a chain of effects is appropriate -when each effect synchronizes with the network (e.g. cascading -dropdowns where each fetches options for the next). Each fetch-bearing -effect has `isExternalSync = true` and is exempt — verified by a -regression test mirroring the `ShippingForm` example from -*Removing Effect Dependencies*. From 93ee8f47a132f753081760411ee2000c3e872337 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 8 May 2026 03:07:33 +0000 Subject: [PATCH 4/7] =?UTF-8?q?fix(react-doctor):=20no-effect-chain=20?= =?UTF-8?q?=E2=80=94=20recognize=20axios.get()=20etc.=20as=20external=20sy?= =?UTF-8?q?nc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot found that EXTERNAL_SYNC_MEMBER_METHOD_NAMES included `post`, `put`, `patch`, `delete`, `fetch` but omitted `get`. Read-only network calls inside an effect (`axios.get(...)`, `ky.get(...)`, `api.get(...)`) were therefore classified as internal-only sync, producing false-positive chain warnings on the article's exact 'cascade of network fetches' exception. Added `get`, `head`, and `options` to round out the standard HTTP method set. Now an `axios.get`-bearing effect correctly has `isExternalSync = true` and the rule won't flag it as a chain. Regression test mirrors the article's ShippingForm example but with `axios.get` instead of `fetch`, asserting zero hits. Co-authored-by: Aiden Bai --- packages/react-doctor/src/plugin/constants.ts | 9 ++- .../tests/regressions/state-rules.test.ts | 55 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/packages/react-doctor/src/plugin/constants.ts b/packages/react-doctor/src/plugin/constants.ts index 59494692..1f0ef3c1 100644 --- a/packages/react-doctor/src/plugin/constants.ts +++ b/packages/react-doctor/src/plugin/constants.ts @@ -367,12 +367,19 @@ export const EXTERNAL_SYNC_MEMBER_METHOD_NAMES = new Set([ "disconnect", "open", "close", - // Network + // Network — `axios.get(...)` etc. need to be recognized as external + // sync just like `axios.post(...)`, otherwise the article's + // legitimate fetch-cascade exception + // (https://react.dev/learn/you-might-not-need-an-effect#chains-of-computations) + // false-positives whenever the cascade is read-only. "fetch", + "get", "post", "put", "patch", "delete", + "head", + "options", ]); export const EXTERNAL_SYNC_DIRECT_CALLEE_NAMES = new Set([ diff --git a/packages/react-doctor/tests/regressions/state-rules.test.ts b/packages/react-doctor/tests/regressions/state-rules.test.ts index 5145b139..f4915d68 100644 --- a/packages/react-doctor/tests/regressions/state-rules.test.ts +++ b/packages/react-doctor/tests/regressions/state-rules.test.ts @@ -378,6 +378,61 @@ export const Chat = ({ roomId }: { roomId: string }) => { expect(hits).toHaveLength(0); }); + it("does NOT flag a fetch-cascade where one effect uses `axios.get` (Bugbot #156)", async () => { + // Regression: previously \`get\` was missing from the external-sync + // member-method allowlist, so an \`axios.get(...)\` effect was + // classified as internal-only. Two such effects with state-flow + // dependence got flagged as a chain even though both were + // legitimately doing network sync. + const projectDir = setupReactProject(tempRoot, "no-effect-chain-axios-get-cascade", { + files: { + "src/Cascade.tsx": `import { useEffect, useState } from "react"; + +declare const axios: { get: (url: string) => Promise<{ data: unknown }> }; + +export const Cascade = ({ country }: { country: string }) => { + const [cities, setCities] = useState(null); + const [city, setCity] = useState(null); + const [areas, setAreas] = useState(null); + + useEffect(() => { + let ignore = false; + axios.get(\`/api/cities?country=\${country}\`).then((response) => { + if (!ignore) setCities(response.data); + }); + return () => { + ignore = true; + }; + }, [country]); + + useEffect(() => { + if (city === null) return; + let ignore = false; + axios.get(\`/api/areas?city=\${city}\`).then((response) => { + if (!ignore) setAreas(response.data); + }); + return () => { + ignore = true; + }; + }, [city]); + + return ( +
+ + {(areas as Array | null)?.length} +
+ ); +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-effect-chain"); + expect(hits).toHaveLength(0); + }); + it("does NOT flag two effects whose written/read state sets are disjoint", async () => { const projectDir = setupReactProject(tempRoot, "no-effect-chain-disjoint", { files: { From 497c7e9993f8e8d57e80a87a2fbf5066e9318794 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 8 May 2026 03:43:43 +0000 Subject: [PATCH 5/7] =?UTF-8?q?fix(react-doctor):=20no-effect-chain=20?= =?UTF-8?q?=E2=80=94=20restrict=20ambiguous=20HTTP=20verbs=20to=20known=20?= =?UTF-8?q?client=20receivers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot caught: round 1 added `get` to EXTERNAL_SYNC_MEMBER_METHOD_NAMES to support `axios.get` cascades, but `get` is also a universal data-structure method name (`Map.get`, `URLSearchParams.get`, `FormData.get`, `Headers.get`, `WeakMap.get`). Effects whose only 'external' call was `params.get('id')` were misclassified as external sync, causing false-NEGATIVE chain detection. Two new constants split the ambiguity: EXTERNAL_SYNC_MEMBER_METHOD_NAMES — unambiguous methods (subscribe, addEventListener, post, put, patch, delete, fetch, …). Receiver is irrelevant; the method name alone marks the call as external. EXTERNAL_SYNC_AMBIGUOUS_HTTP_METHOD_NAMES — the HTTP-vs-data-structure overlapping verbs (get, head, options). Only count as external when paired with a recognized HTTP-client receiver name. EXTERNAL_SYNC_HTTP_CLIENT_RECEIVERS — axios, ky, got, wretch, ofetch, api, client, http, request, fetcher. Regression test asserts that a chain where one effect calls `params.get('theme')` (URLSearchParams) still produces the chain warning, verifying that data-structure-shape `.get` no longer exempts the effect. Co-authored-by: Aiden Bai --- packages/react-doctor/src/plugin/constants.ts | 35 ++++++++++++++----- .../src/plugin/rules/state-and-effects.ts | 30 ++++++++++++---- .../tests/regressions/state-rules.test.ts | 33 +++++++++++++++++ 3 files changed, 84 insertions(+), 14 deletions(-) diff --git a/packages/react-doctor/src/plugin/constants.ts b/packages/react-doctor/src/plugin/constants.ts index 1f0ef3c1..9515de2d 100644 --- a/packages/react-doctor/src/plugin/constants.ts +++ b/packages/react-doctor/src/plugin/constants.ts @@ -353,6 +353,9 @@ export const HOOKS_WITH_DEPS = new Set(["useEffect", "useLayoutEffect", "useMemo // about cascading network fetches) versus pure internal reactivity // (which is the anti-pattern). A cleanup return is the strongest // signal; the curated method list covers the rest. +// Member-method names that, on their own, mark a call as external +// sync regardless of receiver. These are unambiguous in real React +// codebases — they don't clash with built-in JS APIs. export const EXTERNAL_SYNC_MEMBER_METHOD_NAMES = new Set([ // Subscriptions / event listeners "subscribe", @@ -367,21 +370,37 @@ export const EXTERNAL_SYNC_MEMBER_METHOD_NAMES = new Set([ "disconnect", "open", "close", - // Network — `axios.get(...)` etc. need to be recognized as external - // sync just like `axios.post(...)`, otherwise the article's - // legitimate fetch-cascade exception - // (https://react.dev/learn/you-might-not-need-an-effect#chains-of-computations) - // false-positives whenever the cascade is read-only. + // Mutating HTTP verbs — `*.post(url, body)` is essentially always + // a network call. "fetch", - "get", "post", "put", "patch", "delete", - "head", - "options", ]); +// HACK: `get`, `head`, `options` are HTTP verbs but ALSO names of +// universal data-structure methods (`Map.get`, `URLSearchParams.get`, +// `FormData.get`, `Headers.get`, `WeakMap.get`, `Set.has`, etc.). We +// only treat them as external-sync calls when the receiver is a +// recognized HTTP-client-shaped name. Lets the `axios.get(...)` +// cascade case work without false-classifying `params.get('id')` as +// external sync. +export const EXTERNAL_SYNC_HTTP_CLIENT_RECEIVERS = new Set([ + "axios", + "ky", + "got", + "wretch", + "ofetch", + "api", + "client", + "http", + "request", + "fetcher", +]); + +export const EXTERNAL_SYNC_AMBIGUOUS_HTTP_METHOD_NAMES = new Set(["get", "head", "options"]); + export const EXTERNAL_SYNC_DIRECT_CALLEE_NAMES = new Set([ "fetch", "ky", diff --git a/packages/react-doctor/src/plugin/rules/state-and-effects.ts b/packages/react-doctor/src/plugin/rules/state-and-effects.ts index 2914056f..a2a6ac13 100644 --- a/packages/react-doctor/src/plugin/rules/state-and-effects.ts +++ b/packages/react-doctor/src/plugin/rules/state-and-effects.ts @@ -1,7 +1,9 @@ import { CASCADING_SET_STATE_THRESHOLD, EFFECT_HOOK_NAMES, + EXTERNAL_SYNC_AMBIGUOUS_HTTP_METHOD_NAMES, EXTERNAL_SYNC_DIRECT_CALLEE_NAMES, + EXTERNAL_SYNC_HTTP_CLIENT_RECEIVERS, EXTERNAL_SYNC_MEMBER_METHOD_NAMES, EXTERNAL_SYNC_OBSERVER_CONSTRUCTORS, HOOKS_WITH_DEPS, @@ -1274,12 +1276,28 @@ const isExternalSyncEffect = (effectCallback: EsTreeNode): boolean => { return; } - if ( - child.callee?.type === "MemberExpression" && - child.callee.property?.type === "Identifier" && - EXTERNAL_SYNC_MEMBER_METHOD_NAMES.has(child.callee.property.name) - ) { - didFindExternalCall = true; + if (child.callee?.type === "MemberExpression" && child.callee.property?.type === "Identifier") { + const propertyName = child.callee.property.name; + if (EXTERNAL_SYNC_MEMBER_METHOD_NAMES.has(propertyName)) { + didFindExternalCall = true; + return; + } + // HACK: `get` / `head` / `options` are HTTP verbs but also names + // of universal data-structure methods (Map.get, URLSearchParams.get, + // etc.). Only count them when the receiver looks like an HTTP + // client. + if (EXTERNAL_SYNC_AMBIGUOUS_HTTP_METHOD_NAMES.has(propertyName)) { + let receiverCursor: EsTreeNode | undefined = child.callee.object; + while (receiverCursor?.type === "MemberExpression") { + receiverCursor = receiverCursor.object; + } + if ( + receiverCursor?.type === "Identifier" && + EXTERNAL_SYNC_HTTP_CLIENT_RECEIVERS.has(receiverCursor.name) + ) { + didFindExternalCall = true; + } + } } }); diff --git a/packages/react-doctor/tests/regressions/state-rules.test.ts b/packages/react-doctor/tests/regressions/state-rules.test.ts index f4915d68..317f6392 100644 --- a/packages/react-doctor/tests/regressions/state-rules.test.ts +++ b/packages/react-doctor/tests/regressions/state-rules.test.ts @@ -378,6 +378,39 @@ export const Chat = ({ roomId }: { roomId: string }) => { expect(hits).toHaveLength(0); }); + it("DOES still flag chains where effects only call `params.get()` (Bugbot #156 round 2)", async () => { + // Regression: \`get\` as a method name is too ambiguous to count as + // external sync on its own — \`Map.get\`, \`URLSearchParams.get\`, + // \`FormData.get\`, \`Headers.get\` all use the same name. The + // detector now requires the receiver to look like an HTTP client. + // Two effects whose only \"external\" call is \`params.get('id')\` + // should still be classified as internal-only and detected as a + // chain. + const projectDir = setupReactProject(tempRoot, "no-effect-chain-params-get-not-external", { + files: { + "src/Settings.tsx": `import { useEffect, useState } from "react"; + +declare const params: URLSearchParams; + +export const Settings = () => { + const [theme, setTheme] = useState(""); + const [highlight, setHighlight] = useState(""); + useEffect(() => { + setTheme(params.get("theme") ?? "light"); + }, []); + useEffect(() => { + setHighlight(theme === "dark" ? "white" : "black"); + }, [theme]); + return {theme}; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-effect-chain"); + expect(hits.length).toBeGreaterThanOrEqual(1); + }); + it("does NOT flag a fetch-cascade where one effect uses `axios.get` (Bugbot #156)", async () => { // Regression: previously \`get\` was missing from the external-sync // member-method allowlist, so an \`axios.get(...)\` effect was From 161946b3973034c421d28ecbe21cc75c892207df Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 8 May 2026 04:09:42 +0000 Subject: [PATCH 6/7] =?UTF-8?q?fix(react-doctor):=20no-effect-chain=20?= =?UTF-8?q?=E2=80=94=20`delete`=20is=20also=20ambiguous=20(Bugbot=20#156?= =?UTF-8?q?=20round=203)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot caught: round-2 left `delete` in EXTERNAL_SYNC_MEMBER_METHOD_NAMES (unambiguous external-sync) while moving `get`, `head`, `options` to the ambiguous set. But `delete` has the same problem — it's the name of standard JS APIs on Map, Set, URLSearchParams, Headers, FormData, and WeakMap. An effect whose only 'external' call was `set.delete(item)` got falsely classified as external sync, silently exempting it from chain detection. Move `delete` to EXTERNAL_SYNC_AMBIGUOUS_HTTP_METHOD_NAMES so it's only counted as external when paired with a recognized HTTP-client receiver (axios, ky, …). `set.delete()`, `map.delete()`, etc. now correctly stay classified as internal-only. Regression test: a chain where one effect calls `next.delete(...)` on a Set still produces the chain warning. Co-authored-by: Aiden Bai --- packages/react-doctor/src/plugin/constants.ts | 12 +++++-- .../tests/regressions/state-rules.test.ts | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/packages/react-doctor/src/plugin/constants.ts b/packages/react-doctor/src/plugin/constants.ts index 9515de2d..a5b3fb06 100644 --- a/packages/react-doctor/src/plugin/constants.ts +++ b/packages/react-doctor/src/plugin/constants.ts @@ -371,12 +371,13 @@ export const EXTERNAL_SYNC_MEMBER_METHOD_NAMES = new Set([ "open", "close", // Mutating HTTP verbs — `*.post(url, body)` is essentially always - // a network call. + // a network call. (`delete` is moved to the ambiguous set below + // because Map / Set / URLSearchParams / Headers / FormData / + // WeakMap all expose `.delete(...)` as a built-in method.) "fetch", "post", "put", "patch", - "delete", ]); // HACK: `get`, `head`, `options` are HTTP verbs but ALSO names of @@ -399,7 +400,12 @@ export const EXTERNAL_SYNC_HTTP_CLIENT_RECEIVERS = new Set([ "fetcher", ]); -export const EXTERNAL_SYNC_AMBIGUOUS_HTTP_METHOD_NAMES = new Set(["get", "head", "options"]); +export const EXTERNAL_SYNC_AMBIGUOUS_HTTP_METHOD_NAMES = new Set([ + "get", + "head", + "options", + "delete", +]); export const EXTERNAL_SYNC_DIRECT_CALLEE_NAMES = new Set([ "fetch", diff --git a/packages/react-doctor/tests/regressions/state-rules.test.ts b/packages/react-doctor/tests/regressions/state-rules.test.ts index 317f6392..a97ac1d4 100644 --- a/packages/react-doctor/tests/regressions/state-rules.test.ts +++ b/packages/react-doctor/tests/regressions/state-rules.test.ts @@ -378,6 +378,38 @@ export const Chat = ({ roomId }: { roomId: string }) => { expect(hits).toHaveLength(0); }); + it("DOES still flag chains where effects only call `set.delete()` (Bugbot #156 round 3)", async () => { + // Regression: \`delete\` was in the unambiguous external-sync + // method names set, but \`Map.delete\`, \`Set.delete\`, + // \`URLSearchParams.delete\`, etc. all expose the same name. + // Effects that only call data-structure \`.delete\` should still + // be detected as part of an internal-only chain. + const projectDir = setupReactProject(tempRoot, "no-effect-chain-set-delete-not-external", { + files: { + "src/Pruner.tsx": `import { useEffect, useState } from "react"; + +export const Pruner = ({ stale }: { stale: ReadonlySet }) => { + const [pruned, setPruned] = useState>(new Set()); + const [count, setCount] = useState(0); + useEffect(() => { + const next = new Set(); + for (const item of stale) next.add(item); + next.delete("ignore-me"); + setPruned(next); + }, [stale]); + useEffect(() => { + setCount(pruned.size); + }, [pruned]); + return {count}; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "no-effect-chain"); + expect(hits.length).toBeGreaterThanOrEqual(1); + }); + it("DOES still flag chains where effects only call `params.get()` (Bugbot #156 round 2)", async () => { // Regression: \`get\` as a method name is too ambiguous to count as // external sync on its own — \`Map.get\`, \`URLSearchParams.get\`, From c640dfb48996b774e8373b661de12b476492ed99 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 8 May 2026 04:35:43 +0000 Subject: [PATCH 7/7] =?UTF-8?q?test(react-doctor):=20no-effect-chain=20?= =?UTF-8?q?=E2=80=94=20fix=20tests=20so=20they=20actually=20exercise=20isE?= =?UTF-8?q?xternalSync=20(Bugbot=20#156=20round=204)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bugbot caught: the cascade / ShippingForm / chat tests previously had no actual write→dep state overlap between their effects, so they passed regardless of whether `isExternalSync` worked correctly. Reverting the isExternalSync exemption would NOT have caused those tests to fail — they were vacuously green. All three rewritten so the second effect depends on a state that the first effect writes: ShippingForm: writes(A) = {cities}, deps(B) = [cities] axios cascade: writes(A) = {cities}, deps(B) = [cities] chat: writes(A) = {status}, deps(B) = [status] In every case both effects are external-sync (fetch / axios.get / createConnection().connect() / addEventListener), so the chain detector exempts them. If the exemption breaks, all three tests will fail — the chain detection itself fires correctly per the existing intra-effect tests. Co-authored-by: Aiden Bai --- .../tests/regressions/state-rules.test.ts | 99 ++++++++++--------- 1 file changed, 51 insertions(+), 48 deletions(-) diff --git a/packages/react-doctor/tests/regressions/state-rules.test.ts b/packages/react-doctor/tests/regressions/state-rules.test.ts index a97ac1d4..ffa6f785 100644 --- a/packages/react-doctor/tests/regressions/state-rules.test.ts +++ b/packages/react-doctor/tests/regressions/state-rules.test.ts @@ -289,17 +289,18 @@ export const Settings = () => { expect(hits).toHaveLength(0); }); - it("does NOT flag the article's GOOD network-cascade exception", async () => { - // The article explicitly notes that a chain of effects is appropriate - // when each effect synchronizes with the network. Each fetch-bearing - // effect is `isExternalSync = true` and thus exempt. - const projectDir = setupReactProject(tempRoot, "no-effect-chain-network", { + it("does NOT flag the article's GOOD network-cascade exception with a real write→dep chain", async () => { + // CRITICAL: this fixture has writes(A) = {cities} and deps(B) = + // {cities} — a real chain edge between the two effects. Without + // \`isExternalSync\` exempting both, the rule would fire. The + // test only passes if the fetch-bearing effects are correctly + // recognized as external sync. + const projectDir = setupReactProject(tempRoot, "no-effect-chain-network-real-chain", { files: { "src/ShippingForm.tsx": `import { useEffect, useState } from "react"; export const ShippingForm = ({ country }: { country: string }) => { const [cities, setCities] = useState(null); - const [city, setCity] = useState(null); const [areas, setAreas] = useState(null); useEffect(() => { @@ -314,10 +315,12 @@ export const ShippingForm = ({ country }: { country: string }) => { }; }, [country]); + // Real write→dep edge with the previous effect: depends on \`cities\`, + // which the previous effect wrote. Both effects are network sync. useEffect(() => { - if (city === null) return; + if (cities === null) return; let ignore = false; - fetch(\`/api/areas?city=\${city}\`) + fetch(\`/api/areas?cities=\${cities.join(",")}\`) .then((response) => response.json()) .then((json) => { if (!ignore) setAreas(json); @@ -325,14 +328,9 @@ export const ShippingForm = ({ country }: { country: string }) => { return () => { ignore = true; }; - }, [city]); + }, [cities]); - return ( - - ); + return {areas?.length}; }; `, }, @@ -342,33 +340,39 @@ export const ShippingForm = ({ country }: { country: string }) => { expect(hits).toHaveLength(0); }); - it("does NOT flag a chat-connection effect even if it shares deps with another effect", async () => { - // External-system synchronization (createConnection().connect()) sets - // the upstream effect's `isExternalSync = true`, which exempts both - // sides of the would-be edge. - const projectDir = setupReactProject(tempRoot, "no-effect-chain-chat", { + it("does NOT flag a chat-connection chain when both effects do real external sync", async () => { + // CRITICAL: this fixture has Effect A writing \`status\` and + // Effect B depending on \`status\` — a real chain edge. Without + // \`isExternalSync\` exempting the createConnection().connect() + // / disconnect() effect on side A, the rule would fire. + const projectDir = setupReactProject(tempRoot, "no-effect-chain-chat-real-chain", { files: { "src/Chat.tsx": `import { useEffect, useState } from "react"; declare const createConnection: (url: string) => { - connect: () => void; + connect: () => Promise; disconnect: () => void; }; +declare const window: { addEventListener: (name: string, handler: () => void) => void; removeEventListener: (name: string, handler: () => void) => void }; export const Chat = ({ roomId }: { roomId: string }) => { - const [messages, setMessages] = useState([]); + const [status, setStatus] = useState("connecting"); useEffect(() => { const connection = createConnection(roomId); - connection.connect(); + connection.connect().then(setStatus); return () => connection.disconnect(); }, [roomId]); + // Real write→dep edge with the previous effect: depends on \`status\`, + // which Effect A wrote. Effect B also does external sync (DOM listener). useEffect(() => { - setMessages([]); - }, [roomId]); + const onFocus = () => setStatus("connecting"); + window.addEventListener("focus", onFocus); + return () => window.removeEventListener("focus", onFocus); + }, [status]); - return
    {messages.map((line) =>
  • {line}
    • )}
; + return {status}; }; `, }, @@ -443,52 +447,51 @@ export const Settings = () => { expect(hits.length).toBeGreaterThanOrEqual(1); }); - it("does NOT flag a fetch-cascade where one effect uses `axios.get` (Bugbot #156)", async () => { + it("does NOT flag a real write→dep cascade where both effects use `axios.get` (Bugbot #156, real chain)", async () => { // Regression: previously \`get\` was missing from the external-sync - // member-method allowlist, so an \`axios.get(...)\` effect was - // classified as internal-only. Two such effects with state-flow - // dependence got flagged as a chain even though both were - // legitimately doing network sync. - const projectDir = setupReactProject(tempRoot, "no-effect-chain-axios-get-cascade", { + // allowlist, so axios.get() effects were classified as internal- + // only and a real write→dep chain between them got flagged. + // + // CRITICAL: this fixture has an actual write→dep chain. Effect A + // writes \`cities\`; Effect B has \`cities\` in its deps. Without + // \`isExternalSync\` exempting both, the chain detector WOULD fire + // — and previously did before the fix. The test only passes if + // axios.get is recognized as external sync on both effects. + const projectDir = setupReactProject(tempRoot, "no-effect-chain-axios-real-cascade", { files: { "src/Cascade.tsx": `import { useEffect, useState } from "react"; declare const axios: { get: (url: string) => Promise<{ data: unknown }> }; export const Cascade = ({ country }: { country: string }) => { - const [cities, setCities] = useState(null); - const [city, setCity] = useState(null); - const [areas, setAreas] = useState(null); + const [cities, setCities] = useState | null>(null); + const [enriched, setEnriched] = useState | null>(null); useEffect(() => { let ignore = false; axios.get(\`/api/cities?country=\${country}\`).then((response) => { - if (!ignore) setCities(response.data); + if (!ignore) setCities(response.data as Array); }); return () => { ignore = true; }; }, [country]); + // Real chain link: Effect B writes \`enriched\` based on \`cities\` + // (which Effect A wrote). Without isExternalSync, this is a clear + // \`writes(A) ∩ deps(B) = {cities}\` edge. useEffect(() => { - if (city === null) return; + if (cities === null) return; let ignore = false; - axios.get(\`/api/areas?city=\${city}\`).then((response) => { - if (!ignore) setAreas(response.data); + axios.get("/api/enrich").then((response) => { + if (!ignore) setEnriched(response.data as Array); }); return () => { ignore = true; }; - }, [city]); + }, [cities]); - return ( -
- - {(areas as Array | null)?.length} -
- ); + return {enriched?.length}; }; `, },