Skip to content
1 change: 1 addition & 0 deletions packages/react-doctor/src/oxlint-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ export const GLOBAL_REACT_DOCTOR_RULES: Record<string, RuleSeverity> = {
"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",
Expand Down
74 changes: 74 additions & 0 deletions packages/react-doctor/src/plugin/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,80 @@ 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.
// 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",
"addEventListener",
"addListener",
"on",
"watch",
"listen",
"sub",
// Imperative widget lifecycle (createConnection().connect()/.disconnect())
"connect",
"disconnect",
"open",
"close",
// Mutating HTTP verbs — `*.post(url, body)` is essentially always
// a network call.
"fetch",
"post",
"put",
"patch",
"delete",
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
]);
Comment thread
cursor[bot] marked this conversation as resolved.

// 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",
"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"]);

Expand Down
2 changes: 2 additions & 0 deletions packages/react-doctor/src/plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ import {
noDerivedStateEffect,
noDerivedUseState,
noDirectStateMutation,
noEffectChain,
noEffectEventHandler,
noEffectEventInDeps,
noFetchInEffect,
Expand All @@ -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,
Expand Down
223 changes: 223 additions & 0 deletions packages/react-doctor/src/plugin/rules/state-and-effects.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
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,
MUTATING_ARRAY_METHODS,
RELATED_USE_STATE_THRESHOLD,
Expand Down Expand Up @@ -1153,3 +1158,221 @@ 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<string> => {
const depNames = new Set<string>();
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<string, string>,
): Set<string> => {
const writtenStateNames = new Set<string>();
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") {
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;
}
}
}
});

return didFindExternalCall;
};

interface EffectInfo {
node: EsTreeNode;
depNames: Set<string>;
writtenStateNames: Set<string>;
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<string, string>();
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<EsTreeNode>();
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);
},
};
},
};
3 changes: 3 additions & 0 deletions packages/react-doctor/src/utils/run-oxlint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const RULE_CATEGORY_MAP: Record<string, string> = {
"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",
Expand Down Expand Up @@ -240,6 +241,8 @@ const RULE_HELP_MAP: Record<string, string> = {
"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":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,30 @@ const ConditionalSetStateInRenderComponent = ({ count }: { count: number }) => {
return <h1>{prevCount}</h1>;
};

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 (
<div>
{goldCount} {round}
</div>
);
};

const UncontrolledInputComponent = () => {
// HACK: explicit `<string | undefined>` keeps TypeScript happy while the
// RUNTIME initializer stays undefined — that's what trips the
Expand Down Expand Up @@ -169,5 +193,6 @@ export {
DirectStateMutationComponent,
SetStateInRenderComponent,
ConditionalSetStateInRenderComponent,
EffectChainComponent,
UncontrolledInputComponent,
};
Loading
Loading