Skip to content

Commit 945138d

Browse files
feat(react-doctor): gate prefer-use-effect-event behind React 19+ (#162)
Rebased onto main after #153 / #163 merged. Test file reconstructed by appending PR's describe block to main's version + the reactMajorVersion helper-arg modification. All 555 tests pass. Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
1 parent 0be99ad commit 945138d

9 files changed

Lines changed: 669 additions & 1 deletion

File tree

packages/react-doctor/src/constants.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,7 @@ export const JSX_OPENER_SCAN_MAX_LINES = 32;
9696
// HACK: lookback cap for stacked / near-miss disable-next-line scanning.
9797
// Larger gaps stop being intentional suppressions and become noise.
9898
export const SUPPRESSION_NEAR_MISS_MAX_LINES = 10;
99+
100+
// `useEffectEvent` requires React 19+. Below the threshold, the rule
101+
// that suggests it (`prefer-use-effect-event`) stays silent.
102+
export const USE_EFFECT_EVENT_MIN_MAJOR = 19;

packages/react-doctor/src/oxlint-config.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { createRequire } from "node:module";
2-
import { REACT_19_DEPRECATION_MIN_MAJOR, REACT_DOM_LEGACY_API_MIN_MAJOR } from "./constants.js";
2+
import {
3+
REACT_19_DEPRECATION_MIN_MAJOR,
4+
REACT_DOM_LEGACY_API_MIN_MAJOR,
5+
USE_EFFECT_EVENT_MIN_MAJOR,
6+
} from "./constants.js";
37
import type { Framework } from "./types.js";
48

59
const esmRequire = createRequire(import.meta.url);
@@ -241,6 +245,7 @@ export const GLOBAL_REACT_DOCTOR_RULES: Record<string, RuleSeverity> = {
241245
"react-doctor/no-derived-useState": "warn",
242246
"react-doctor/no-direct-state-mutation": "warn",
243247
"react-doctor/no-set-state-in-render": "warn",
248+
"react-doctor/prefer-use-effect-event": "warn",
244249
"react-doctor/prefer-useReducer": "warn",
245250
"react-doctor/prefer-use-sync-external-store": "warn",
246251
"react-doctor/rerender-lazy-state-init": "warn",
@@ -382,6 +387,7 @@ const VERSION_GATED_RULE_IDS: ReadonlyMap<string, number> = new Map([
382387
["react-doctor/no-react19-deprecated-apis", REACT_19_DEPRECATION_MIN_MAJOR],
383388
["react-doctor/no-default-props", REACT_19_DEPRECATION_MIN_MAJOR],
384389
["react-doctor/no-react-dom-deprecated-apis", REACT_DOM_LEGACY_API_MIN_MAJOR],
390+
["react-doctor/prefer-use-effect-event", USE_EFFECT_EVENT_MIN_MAJOR],
385391
]);
386392

387393
const filterRulesByReactMajor = (

packages/react-doctor/src/plugin/constants.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,18 @@ export const MUTATING_ROUTE_SEGMENTS = new Set([
390390
export const EFFECT_HOOK_NAMES = new Set(["useEffect", "useLayoutEffect"]);
391391
export const HOOKS_WITH_DEPS = new Set(["useEffect", "useLayoutEffect", "useMemo", "useCallback"]);
392392

393+
// Direct CallExpression callees whose function argument is a "sub-handler"
394+
// — code that runs asynchronously, in response to an event the React render
395+
// can't observe. Calling a reactive value from inside a sub-handler is the
396+
// classic case for `useEffectEvent` (see "Separating Events from Effects").
397+
export const SUB_HANDLER_DIRECT_CALLEE_NAMES = new Set([
398+
"setTimeout",
399+
"setInterval",
400+
"requestAnimationFrame",
401+
"requestIdleCallback",
402+
"queueMicrotask",
403+
]);
404+
393405
// Globals whose values mutate outside the React data flow. Listing
394406
// them as deps doesn't trigger a re-run when they change because
395407
// React compares deps with `Object.is` during render — and the read

packages/react-doctor/src/plugin/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ import {
190190
noMutableInDeps,
191191
noPropCallbackInEffect,
192192
noSetStateInRender,
193+
preferUseEffectEvent,
193194
preferUseReducer,
194195
preferUseSyncExternalStore,
195196
rerenderDependencies,
@@ -217,6 +218,7 @@ const plugin: RulePlugin = {
217218
"no-derived-useState": noDerivedUseState,
218219
"no-direct-state-mutation": noDirectStateMutation,
219220
"no-set-state-in-render": noSetStateInRender,
221+
"prefer-use-effect-event": preferUseEffectEvent,
220222
"prefer-useReducer": preferUseReducer,
221223
"prefer-use-sync-external-store": preferUseSyncExternalStore,
222224
"rerender-lazy-state-init": rerenderLazyStateInit,

packages/react-doctor/src/plugin/rules/state-and-effects.ts

Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
MUTABLE_GLOBAL_ROOTS,
1414
MUTATING_ARRAY_METHODS,
1515
RELATED_USE_STATE_THRESHOLD,
16+
SUB_HANDLER_DIRECT_CALLEE_NAMES,
1617
SUBSCRIPTION_METHOD_NAMES,
1718
TRIVIAL_DERIVATION_CALLEE_NAMES,
1819
TRIVIAL_INITIALIZER_NAMES,
@@ -2554,3 +2555,276 @@ export const noMutableInDeps: Rule = {
25542555
};
25552556
},
25562557
};
2558+
2559+
// HACK: From "Separating Events from Effects" — when a function-typed
2560+
// prop (or local callback) is read from an effect ONLY inside a sub-
2561+
// handler (setTimeout / addEventListener / store.subscribe / etc.),
2562+
// listing it in the dep array forces the whole effect to re-synchronize
2563+
// every time its identity changes. The article's recommended fix is
2564+
// `useEffectEvent`, which is React 19+. The rule is registered as
2565+
// version-gated in `oxlint-config.ts` (USE_EFFECT_EVENT_MIN_MAJOR) so
2566+
// pre-19 projects don't see noisy diagnostics for an API they don't
2567+
// have.
2568+
//
2569+
// function SearchInput({ onSearch }) {
2570+
// const [query, setQuery] = useState('');
2571+
// useEffect(() => {
2572+
// const id = setTimeout(() => onSearch(query), 300); // sub-handler
2573+
// return () => clearTimeout(id);
2574+
// }, [query, onSearch]);
2575+
// }
2576+
//
2577+
// Detector pre-conditions (all must hold) — chosen to keep FPs near zero:
2578+
// (1) useEffect with at least 2 dep array elements, all Identifiers
2579+
// (2) at least one dep `F` is a function-shaped reactive value:
2580+
// - a destructured prop named `on[A-Z]…`, OR
2581+
// - a local declared via `const F = useCallback(...)`
2582+
// (3) every read of `F` inside the effect body sits inside a sub-
2583+
// handler (SUB_HANDLER_DIRECT_CALLEE_NAMES, OR a MemberExpression
2584+
// whose property is in SUBSCRIPTION_METHOD_NAMES — same set the
2585+
// prefer-use-sync-external-store family uses)
2586+
// (4) `F` is NEVER read at the effect's own top level
2587+
const collectFunctionTypedLocalBindings = (componentBody: EsTreeNode): Set<string> => {
2588+
const functionTypedLocals = new Set<string>();
2589+
if (componentBody?.type !== "BlockStatement") return functionTypedLocals;
2590+
for (const statement of componentBody.body ?? []) {
2591+
if (statement.type !== "VariableDeclaration") continue;
2592+
for (const declarator of statement.declarations ?? []) {
2593+
if (declarator.id?.type !== "Identifier") continue;
2594+
if (declarator.init?.type !== "CallExpression") continue;
2595+
if (!isHookCall(declarator.init, "useCallback")) continue;
2596+
functionTypedLocals.add(declarator.id.name);
2597+
}
2598+
}
2599+
return functionTypedLocals;
2600+
};
2601+
2602+
const findEnclosingFunctionInsideEffect = (
2603+
identifierNode: EsTreeNode,
2604+
effectCallback: EsTreeNode,
2605+
): EsTreeNode | null => {
2606+
let cursor: EsTreeNode | null = identifierNode.parent ?? null;
2607+
while (cursor && cursor !== effectCallback) {
2608+
if (
2609+
cursor.type === "ArrowFunctionExpression" ||
2610+
cursor.type === "FunctionExpression" ||
2611+
cursor.type === "FunctionDeclaration"
2612+
) {
2613+
return cursor;
2614+
}
2615+
cursor = cursor.parent ?? null;
2616+
}
2617+
return null;
2618+
};
2619+
2620+
const isCallExpressionWithSubHandlerCallee = (callExpression: EsTreeNode): boolean => {
2621+
if (callExpression?.type !== "CallExpression") return false;
2622+
const callee = callExpression.callee;
2623+
if (callee?.type === "Identifier" && SUB_HANDLER_DIRECT_CALLEE_NAMES.has(callee.name)) {
2624+
return true;
2625+
}
2626+
if (
2627+
callee?.type === "MemberExpression" &&
2628+
callee.property?.type === "Identifier" &&
2629+
SUBSCRIPTION_METHOD_NAMES.has(callee.property.name)
2630+
) {
2631+
return true;
2632+
}
2633+
return false;
2634+
};
2635+
2636+
const getSubHandlerCalleeName = (callExpression: EsTreeNode): string | null => {
2637+
if (callExpression?.type !== "CallExpression") return null;
2638+
const callee = callExpression.callee;
2639+
if (callee?.type === "Identifier") return callee.name;
2640+
if (callee?.type === "MemberExpression" && callee.property?.type === "Identifier") {
2641+
return callee.property.name;
2642+
}
2643+
return null;
2644+
};
2645+
2646+
// HACK: handles the dominant real-world shape where the handler is
2647+
// bound to a const before being passed to addEventListener / subscribe:
2648+
//
2649+
// const handler = (event) => onKey(event.key);
2650+
// window.addEventListener('keydown', handler);
2651+
// return () => window.removeEventListener('keydown', handler);
2652+
//
2653+
// Walks up to the function-level node (the arrow expression) and checks
2654+
// for either a direct sub-handler argument position OR a const binding
2655+
// whose Identifier appears as an argument to a sub-handler call later
2656+
// in the same effect body.
2657+
const findSubHandlerForEnclosingFunction = (
2658+
enclosingFunction: EsTreeNode,
2659+
effectCallback: EsTreeNode,
2660+
): EsTreeNode | null => {
2661+
const directParent = enclosingFunction.parent;
2662+
if (
2663+
directParent?.type === "CallExpression" &&
2664+
directParent.arguments?.includes(enclosingFunction) &&
2665+
isCallExpressionWithSubHandlerCallee(directParent)
2666+
) {
2667+
return directParent;
2668+
}
2669+
2670+
if (directParent?.type !== "VariableDeclarator") return null;
2671+
if (directParent.id?.type !== "Identifier") return null;
2672+
const localName: string = directParent.id.name;
2673+
2674+
let matchingSubHandlerCall: EsTreeNode | null = null;
2675+
walkAst(effectCallback, (child: EsTreeNode) => {
2676+
if (matchingSubHandlerCall) return false;
2677+
if (child.type !== "CallExpression") return;
2678+
if (!isCallExpressionWithSubHandlerCallee(child)) return;
2679+
for (const argument of child.arguments ?? []) {
2680+
if (argument?.type === "Identifier" && argument.name === localName) {
2681+
matchingSubHandlerCall = child;
2682+
return false;
2683+
}
2684+
}
2685+
});
2686+
return matchingSubHandlerCall;
2687+
};
2688+
2689+
interface CallableReadClassification {
2690+
hasAnyRead: boolean;
2691+
allReadsAreInSubHandlers: boolean;
2692+
firstSubHandlerName: string | null;
2693+
}
2694+
2695+
const classifyCallableReadsInsideEffect = (
2696+
callableName: string,
2697+
effectCallback: EsTreeNode,
2698+
): CallableReadClassification => {
2699+
let hasAnyRead = false;
2700+
let allReadsAreInSubHandlers = true;
2701+
let firstSubHandlerName: string | null = null;
2702+
2703+
walkAst(effectCallback, (child: EsTreeNode) => {
2704+
if (child.type !== "Identifier") return;
2705+
if (child.name !== callableName) return;
2706+
const parent = child.parent;
2707+
if (parent?.type === "ArrayExpression") return;
2708+
if (parent?.type === "MemberExpression" && !parent.computed && parent.property === child) {
2709+
return;
2710+
}
2711+
if (
2712+
parent?.type === "Property" &&
2713+
!parent.computed &&
2714+
!parent.shorthand &&
2715+
parent.key === child
2716+
) {
2717+
return;
2718+
}
2719+
2720+
hasAnyRead = true;
2721+
2722+
const enclosingFunction = findEnclosingFunctionInsideEffect(child, effectCallback);
2723+
if (!enclosingFunction) {
2724+
allReadsAreInSubHandlers = false;
2725+
return;
2726+
}
2727+
const subHandlerCall = findSubHandlerForEnclosingFunction(enclosingFunction, effectCallback);
2728+
if (!subHandlerCall) {
2729+
allReadsAreInSubHandlers = false;
2730+
return;
2731+
}
2732+
if (firstSubHandlerName === null) {
2733+
firstSubHandlerName = getSubHandlerCalleeName(subHandlerCall);
2734+
}
2735+
});
2736+
2737+
return { hasAnyRead, allReadsAreInSubHandlers, firstSubHandlerName };
2738+
};
2739+
2740+
export const preferUseEffectEvent: Rule = {
2741+
create: (context: RuleContext) => {
2742+
const componentPropParamStack: Array<Set<string>> = [];
2743+
2744+
const isPropName = (name: string): boolean => {
2745+
for (let frameIndex = componentPropParamStack.length - 1; frameIndex >= 0; frameIndex--) {
2746+
const frame = componentPropParamStack[frameIndex];
2747+
if (frame.size === 0) return false;
2748+
if (frame.has(name)) return true;
2749+
}
2750+
return false;
2751+
};
2752+
2753+
const checkComponent = (componentBody: EsTreeNode | null | undefined): void => {
2754+
if (!componentBody || componentBody.type !== "BlockStatement") return;
2755+
const functionTypedLocalBindings = collectFunctionTypedLocalBindings(componentBody);
2756+
2757+
for (const statement of componentBody.body ?? []) {
2758+
if (statement.type !== "ExpressionStatement") continue;
2759+
const effectCall = statement.expression;
2760+
if (effectCall?.type !== "CallExpression") continue;
2761+
if (!isHookCall(effectCall, EFFECT_HOOK_NAMES)) continue;
2762+
if ((effectCall.arguments?.length ?? 0) < 2) continue;
2763+
2764+
const depsNode = effectCall.arguments[1];
2765+
if (depsNode.type !== "ArrayExpression") continue;
2766+
const depElements = depsNode.elements ?? [];
2767+
if (depElements.length < 2) continue;
2768+
if (!depElements.every((element: EsTreeNode | null) => element?.type === "Identifier")) {
2769+
continue;
2770+
}
2771+
2772+
const callback = getEffectCallback(effectCall);
2773+
if (!callback) continue;
2774+
2775+
for (const depElement of depElements) {
2776+
if (!depElement) continue;
2777+
const depName: string = depElement.name;
2778+
// HACK: a destructured prop is treated as function-typed
2779+
// ONLY if its name matches the React `on[A-Z]` callback
2780+
// convention. Without this filter the rule false-positived
2781+
// on scalar props.
2782+
const isFunctionTypedPropDep = isPropName(depName) && /^on[A-Z]/.test(depName);
2783+
const isFunctionTypedLocalDep = functionTypedLocalBindings.has(depName);
2784+
if (!isFunctionTypedPropDep && !isFunctionTypedLocalDep) continue;
2785+
2786+
const classification = classifyCallableReadsInsideEffect(depName, callback);
2787+
if (!classification.hasAnyRead) continue;
2788+
if (!classification.allReadsAreInSubHandlers) continue;
2789+
2790+
const subHandlerLabel = classification.firstSubHandlerName
2791+
? `\`${classification.firstSubHandlerName}\``
2792+
: "an async sub-handler";
2793+
context.report({
2794+
node: depElement,
2795+
message: `"${depName}" is read only inside ${subHandlerLabel} — wrap it with useEffectEvent and remove it from the dep array so the effect doesn't re-synchronize on every parent render`,
2796+
});
2797+
}
2798+
}
2799+
};
2800+
2801+
return {
2802+
FunctionDeclaration(node: EsTreeNode) {
2803+
if (!node.id?.name || !isUppercaseName(node.id.name)) {
2804+
componentPropParamStack.push(new Set());
2805+
return;
2806+
}
2807+
componentPropParamStack.push(extractDestructuredPropNames(node.params ?? []));
2808+
checkComponent(node.body);
2809+
},
2810+
"FunctionDeclaration:exit"() {
2811+
componentPropParamStack.pop();
2812+
},
2813+
VariableDeclarator(node: EsTreeNode) {
2814+
if (isComponentAssignment(node)) {
2815+
componentPropParamStack.push(extractDestructuredPropNames(node.init?.params ?? []));
2816+
checkComponent(node.init?.body);
2817+
return;
2818+
}
2819+
if (isFunctionLikeVariableDeclarator(node)) {
2820+
componentPropParamStack.push(new Set());
2821+
}
2822+
},
2823+
"VariableDeclarator:exit"(node: EsTreeNode) {
2824+
if (isComponentAssignment(node) || isFunctionLikeVariableDeclarator(node)) {
2825+
componentPropParamStack.pop();
2826+
}
2827+
},
2828+
};
2829+
},
2830+
};

packages/react-doctor/src/utils/run-oxlint.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ const RULE_CATEGORY_MAP: Record<string, string> = {
5656
"react-doctor/no-derived-useState": "State & Effects",
5757
"react-doctor/no-direct-state-mutation": "State & Effects",
5858
"react-doctor/no-set-state-in-render": "State & Effects",
59+
"react-doctor/prefer-use-effect-event": "State & Effects",
5960
"react-doctor/prefer-useReducer": "State & Effects",
6061
"react-doctor/prefer-use-sync-external-store": "State & Effects",
6162
"react-doctor/rerender-lazy-state-init": "Performance",
@@ -262,6 +263,8 @@ const RULE_HELP_MAP: Record<string, string> = {
262263
"Replace the mutation with a setter call that produces a new reference: `setItems([...items, newItem])`, `setItems(items.filter(x => x !== target))`, `setItems(items.toSorted(...))`. React only re-renders on a new reference, so in-place updates are silently dropped",
263264
"no-set-state-in-render":
264265
"Move the setter call into a `useEffect`, an event handler, or replace the state with a value computed during render. Calling a setter at render time triggers another render, which calls the setter again — an infinite loop",
266+
"prefer-use-effect-event":
267+
"Wrap the callback with `useEffectEvent(callback)` (React 19+) and call the resulting binding from inside the sub-handler. The Effect Event captures the latest props/state without being a reactive dep, so the effect doesn't re-subscribe on every parent render. See https://react.dev/reference/react/useEffectEvent",
265268
"prefer-useReducer":
266269
"Group related state: `const [state, dispatch] = useReducer(reducer, { field1, field2, ... })`",
267270
"prefer-use-sync-external-store":

packages/react-doctor/tests/fixtures/basic-react/src/state-issues.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,15 @@ const MutableInDepsComponent = ({ token }: { token: string }) => {
159159
return <div />;
160160
};
161161

162+
const PreferUseEffectEventComponent = ({ onSearch }: { onSearch: (q: string) => void }) => {
163+
const [query, setQuery] = useState("");
164+
useEffect(() => {
165+
const id = setTimeout(() => onSearch(query), 300);
166+
return () => clearTimeout(id);
167+
}, [query, onSearch]);
168+
return <input value={query} onChange={(event) => setQuery(event.target.value)} />;
169+
};
170+
162171
declare const externalStore: {
163172
subscribe: (listener: () => void) => () => void;
164173
getSnapshot: () => number;
@@ -258,6 +267,7 @@ export {
258267
EffectNeedsCleanupComponent,
259268
MirrorPropEffectComponent,
260269
MutableInDepsComponent,
270+
PreferUseEffectEventComponent,
261271
SubscribeStorePatternComponent,
262272
EventTriggerStateComponent,
263273
EffectChainComponent,

0 commit comments

Comments
 (0)