Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/react-doctor/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,7 @@ export const JSX_OPENER_SCAN_MAX_LINES = 32;
// HACK: lookback cap for stacked / near-miss disable-next-line scanning.
// Larger gaps stop being intentional suppressions and become noise.
export const SUPPRESSION_NEAR_MISS_MAX_LINES = 10;

// `useEffectEvent` requires React 19+. Below the threshold, the rule
// that suggests it (`prefer-use-effect-event`) stays silent.
export const USE_EFFECT_EVENT_MIN_MAJOR = 19;
8 changes: 7 additions & 1 deletion packages/react-doctor/src/oxlint-config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { createRequire } from "node:module";
import { REACT_19_DEPRECATION_MIN_MAJOR, REACT_DOM_LEGACY_API_MIN_MAJOR } from "./constants.js";
import {
REACT_19_DEPRECATION_MIN_MAJOR,
REACT_DOM_LEGACY_API_MIN_MAJOR,
USE_EFFECT_EVENT_MIN_MAJOR,
} from "./constants.js";
import type { Framework } from "./types.js";

const esmRequire = createRequire(import.meta.url);
Expand Down Expand Up @@ -241,6 +245,7 @@ export const GLOBAL_REACT_DOCTOR_RULES: Record<string, RuleSeverity> = {
"react-doctor/no-derived-useState": "warn",
"react-doctor/no-direct-state-mutation": "warn",
"react-doctor/no-set-state-in-render": "warn",
"react-doctor/prefer-use-effect-event": "warn",
Comment thread
cursor[bot] marked this conversation as resolved.
"react-doctor/prefer-useReducer": "warn",
"react-doctor/prefer-use-sync-external-store": "warn",
"react-doctor/rerender-lazy-state-init": "warn",
Expand Down Expand Up @@ -382,6 +387,7 @@ const VERSION_GATED_RULE_IDS: ReadonlyMap<string, number> = new Map([
["react-doctor/no-react19-deprecated-apis", REACT_19_DEPRECATION_MIN_MAJOR],
["react-doctor/no-default-props", REACT_19_DEPRECATION_MIN_MAJOR],
["react-doctor/no-react-dom-deprecated-apis", REACT_DOM_LEGACY_API_MIN_MAJOR],
["react-doctor/prefer-use-effect-event", USE_EFFECT_EVENT_MIN_MAJOR],
]);

const filterRulesByReactMajor = (
Expand Down
12 changes: 12 additions & 0 deletions packages/react-doctor/src/plugin/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,18 @@ 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"]);

// Direct CallExpression callees whose function argument is a "sub-handler"
// β€” code that runs asynchronously, in response to an event the React render
// can't observe. Calling a reactive value from inside a sub-handler is the
// classic case for `useEffectEvent` (see "Separating Events from Effects").
export const SUB_HANDLER_DIRECT_CALLEE_NAMES = new Set([
"setTimeout",
"setInterval",
"requestAnimationFrame",
"requestIdleCallback",
"queueMicrotask",
]);

// Globals whose values mutate outside the React data flow. Listing
// them as deps doesn't trigger a re-run when they change because
// React compares deps with `Object.is` during render β€” and the read
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 @@ -190,6 +190,7 @@ import {
noMutableInDeps,
noPropCallbackInEffect,
noSetStateInRender,
preferUseEffectEvent,
preferUseReducer,
preferUseSyncExternalStore,
rerenderDependencies,
Expand Down Expand Up @@ -217,6 +218,7 @@ const plugin: RulePlugin = {
"no-derived-useState": noDerivedUseState,
"no-direct-state-mutation": noDirectStateMutation,
"no-set-state-in-render": noSetStateInRender,
"prefer-use-effect-event": preferUseEffectEvent,
"prefer-useReducer": preferUseReducer,
"prefer-use-sync-external-store": preferUseSyncExternalStore,
"rerender-lazy-state-init": rerenderLazyStateInit,
Expand Down
274 changes: 274 additions & 0 deletions packages/react-doctor/src/plugin/rules/state-and-effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
MUTABLE_GLOBAL_ROOTS,
MUTATING_ARRAY_METHODS,
RELATED_USE_STATE_THRESHOLD,
SUB_HANDLER_DIRECT_CALLEE_NAMES,
SUBSCRIPTION_METHOD_NAMES,
TRIVIAL_DERIVATION_CALLEE_NAMES,
TRIVIAL_INITIALIZER_NAMES,
Expand Down Expand Up @@ -2554,3 +2555,276 @@ export const noMutableInDeps: Rule = {
};
},
};

// HACK: From "Separating Events from Effects" β€” when a function-typed
// prop (or local callback) is read from an effect ONLY inside a sub-
// handler (setTimeout / addEventListener / store.subscribe / etc.),
// listing it in the dep array forces the whole effect to re-synchronize
// every time its identity changes. The article's recommended fix is
// `useEffectEvent`, which is React 19+. The rule is registered as
// version-gated in `oxlint-config.ts` (USE_EFFECT_EVENT_MIN_MAJOR) so
// pre-19 projects don't see noisy diagnostics for an API they don't
// have.
//
// function SearchInput({ onSearch }) {
// const [query, setQuery] = useState('');
// useEffect(() => {
// const id = setTimeout(() => onSearch(query), 300); // sub-handler
// return () => clearTimeout(id);
// }, [query, onSearch]);
// }
//
// Detector pre-conditions (all must hold) β€” chosen to keep FPs near zero:
// (1) useEffect with at least 2 dep array elements, all Identifiers
// (2) at least one dep `F` is a function-shaped reactive value:
// - a destructured prop named `on[A-Z]…`, OR
// - a local declared via `const F = useCallback(...)`
// (3) every read of `F` inside the effect body sits inside a sub-
// handler (SUB_HANDLER_DIRECT_CALLEE_NAMES, OR a MemberExpression
// whose property is in SUBSCRIPTION_METHOD_NAMES β€” same set the
// prefer-use-sync-external-store family uses)
// (4) `F` is NEVER read at the effect's own top level
const collectFunctionTypedLocalBindings = (componentBody: EsTreeNode): Set<string> => {
const functionTypedLocals = new Set<string>();
if (componentBody?.type !== "BlockStatement") return functionTypedLocals;
for (const statement of componentBody.body ?? []) {
if (statement.type !== "VariableDeclaration") continue;
for (const declarator of statement.declarations ?? []) {
if (declarator.id?.type !== "Identifier") continue;
if (declarator.init?.type !== "CallExpression") continue;
if (!isHookCall(declarator.init, "useCallback")) continue;
functionTypedLocals.add(declarator.id.name);
}
}
return functionTypedLocals;
};

const findEnclosingFunctionInsideEffect = (
identifierNode: EsTreeNode,
effectCallback: EsTreeNode,
): EsTreeNode | null => {
let cursor: EsTreeNode | null = identifierNode.parent ?? null;
while (cursor && cursor !== effectCallback) {
if (
cursor.type === "ArrowFunctionExpression" ||
cursor.type === "FunctionExpression" ||
cursor.type === "FunctionDeclaration"
) {
return cursor;
}
cursor = cursor.parent ?? null;
}
return null;
};

const isCallExpressionWithSubHandlerCallee = (callExpression: EsTreeNode): boolean => {
if (callExpression?.type !== "CallExpression") return false;
const callee = callExpression.callee;
if (callee?.type === "Identifier" && SUB_HANDLER_DIRECT_CALLEE_NAMES.has(callee.name)) {
return true;
}
if (
callee?.type === "MemberExpression" &&
callee.property?.type === "Identifier" &&
SUBSCRIPTION_METHOD_NAMES.has(callee.property.name)
) {
return true;
}
return false;
};

const getSubHandlerCalleeName = (callExpression: EsTreeNode): string | null => {
if (callExpression?.type !== "CallExpression") return null;
const callee = callExpression.callee;
if (callee?.type === "Identifier") return callee.name;
if (callee?.type === "MemberExpression" && callee.property?.type === "Identifier") {
return callee.property.name;
}
return null;
};

// HACK: handles the dominant real-world shape where the handler is
// bound to a const before being passed to addEventListener / subscribe:
//
// const handler = (event) => onKey(event.key);
// window.addEventListener('keydown', handler);
// return () => window.removeEventListener('keydown', handler);
//
// Walks up to the function-level node (the arrow expression) and checks
// for either a direct sub-handler argument position OR a const binding
// whose Identifier appears as an argument to a sub-handler call later
// in the same effect body.
const findSubHandlerForEnclosingFunction = (
enclosingFunction: EsTreeNode,
effectCallback: EsTreeNode,
): EsTreeNode | null => {
const directParent = enclosingFunction.parent;
if (
directParent?.type === "CallExpression" &&
directParent.arguments?.includes(enclosingFunction) &&
isCallExpressionWithSubHandlerCallee(directParent)
) {
return directParent;
}

if (directParent?.type !== "VariableDeclarator") return null;
if (directParent.id?.type !== "Identifier") return null;
const localName: string = directParent.id.name;

let matchingSubHandlerCall: EsTreeNode | null = null;
walkAst(effectCallback, (child: EsTreeNode) => {
if (matchingSubHandlerCall) return false;
if (child.type !== "CallExpression") return;
if (!isCallExpressionWithSubHandlerCallee(child)) return;
for (const argument of child.arguments ?? []) {
if (argument?.type === "Identifier" && argument.name === localName) {
matchingSubHandlerCall = child;
return false;
}
}
});
return matchingSubHandlerCall;
};

interface CallableReadClassification {
hasAnyRead: boolean;
allReadsAreInSubHandlers: boolean;
firstSubHandlerName: string | null;
}

const classifyCallableReadsInsideEffect = (
callableName: string,
effectCallback: EsTreeNode,
): CallableReadClassification => {
let hasAnyRead = false;
let allReadsAreInSubHandlers = true;
let firstSubHandlerName: string | null = null;

walkAst(effectCallback, (child: EsTreeNode) => {
if (child.type !== "Identifier") return;
if (child.name !== callableName) return;
const parent = child.parent;
if (parent?.type === "ArrayExpression") return;
Comment thread
cursor[bot] marked this conversation as resolved.
if (parent?.type === "MemberExpression" && !parent.computed && parent.property === child) {
return;
}
if (
parent?.type === "Property" &&
!parent.computed &&
!parent.shorthand &&
parent.key === child
) {
return;
}

hasAnyRead = true;

const enclosingFunction = findEnclosingFunctionInsideEffect(child, effectCallback);
if (!enclosingFunction) {
allReadsAreInSubHandlers = false;
return;
}
const subHandlerCall = findSubHandlerForEnclosingFunction(enclosingFunction, effectCallback);
if (!subHandlerCall) {
allReadsAreInSubHandlers = false;
return;
}
if (firstSubHandlerName === null) {
firstSubHandlerName = getSubHandlerCalleeName(subHandlerCall);
}
});

return { hasAnyRead, allReadsAreInSubHandlers, firstSubHandlerName };
};

export const preferUseEffectEvent: Rule = {
create: (context: RuleContext) => {
const componentPropParamStack: Array<Set<string>> = [];

const isPropName = (name: string): boolean => {
for (let frameIndex = componentPropParamStack.length - 1; frameIndex >= 0; frameIndex--) {
const frame = componentPropParamStack[frameIndex];
if (frame.size === 0) return false;
if (frame.has(name)) return true;
}
return false;
};

const checkComponent = (componentBody: EsTreeNode | null | undefined): void => {
if (!componentBody || componentBody.type !== "BlockStatement") return;
const functionTypedLocalBindings = collectFunctionTypedLocalBindings(componentBody);

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

const depsNode = effectCall.arguments[1];
if (depsNode.type !== "ArrayExpression") continue;
const depElements = depsNode.elements ?? [];
if (depElements.length < 2) continue;
if (!depElements.every((element: EsTreeNode | null) => element?.type === "Identifier")) {
continue;
}

const callback = getEffectCallback(effectCall);
if (!callback) continue;

for (const depElement of depElements) {
if (!depElement) continue;
const depName: string = depElement.name;
// HACK: a destructured prop is treated as function-typed
// ONLY if its name matches the React `on[A-Z]` callback
// convention. Without this filter the rule false-positived
// on scalar props.
const isFunctionTypedPropDep = isPropName(depName) && /^on[A-Z]/.test(depName);
const isFunctionTypedLocalDep = functionTypedLocalBindings.has(depName);
if (!isFunctionTypedPropDep && !isFunctionTypedLocalDep) continue;

const classification = classifyCallableReadsInsideEffect(depName, callback);
if (!classification.hasAnyRead) continue;
if (!classification.allReadsAreInSubHandlers) continue;

const subHandlerLabel = classification.firstSubHandlerName
? `\`${classification.firstSubHandlerName}\``
: "an async sub-handler";
context.report({
node: depElement,
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`,
});
}
}
};

return {
FunctionDeclaration(node: EsTreeNode) {
if (!node.id?.name || !isUppercaseName(node.id.name)) {
componentPropParamStack.push(new Set());
return;
}
componentPropParamStack.push(extractDestructuredPropNames(node.params ?? []));
checkComponent(node.body);
},
"FunctionDeclaration:exit"() {
componentPropParamStack.pop();
},
VariableDeclarator(node: EsTreeNode) {
if (isComponentAssignment(node)) {
componentPropParamStack.push(extractDestructuredPropNames(node.init?.params ?? []));
checkComponent(node.init?.body);
return;
}
if (isFunctionLikeVariableDeclarator(node)) {
componentPropParamStack.push(new Set());
}
},
"VariableDeclarator:exit"(node: EsTreeNode) {
if (isComponentAssignment(node) || isFunctionLikeVariableDeclarator(node)) {
componentPropParamStack.pop();
}
},
};
},
};
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 @@ -56,6 +56,7 @@ const RULE_CATEGORY_MAP: Record<string, string> = {
"react-doctor/no-derived-useState": "State & Effects",
"react-doctor/no-direct-state-mutation": "State & Effects",
"react-doctor/no-set-state-in-render": "State & Effects",
"react-doctor/prefer-use-effect-event": "State & Effects",
"react-doctor/prefer-useReducer": "State & Effects",
"react-doctor/prefer-use-sync-external-store": "State & Effects",
"react-doctor/rerender-lazy-state-init": "Performance",
Expand Down Expand Up @@ -262,6 +263,8 @@ const RULE_HELP_MAP: Record<string, string> = {
"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",
"no-set-state-in-render":
"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",
"prefer-use-effect-event":
"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",
"prefer-useReducer":
"Group related state: `const [state, dispatch] = useReducer(reducer, { field1, field2, ... })`",
"prefer-use-sync-external-store":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,15 @@ const MutableInDepsComponent = ({ token }: { token: string }) => {
return <div />;
};

const PreferUseEffectEventComponent = ({ onSearch }: { onSearch: (q: string) => void }) => {
const [query, setQuery] = useState("");
useEffect(() => {
const id = setTimeout(() => onSearch(query), 300);
return () => clearTimeout(id);
}, [query, onSearch]);
return <input value={query} onChange={(event) => setQuery(event.target.value)} />;
};

declare const externalStore: {
subscribe: (listener: () => void) => () => void;
getSnapshot: () => number;
Expand Down Expand Up @@ -258,6 +267,7 @@ export {
EffectNeedsCleanupComponent,
MirrorPropEffectComponent,
MutableInDepsComponent,
PreferUseEffectEventComponent,
SubscribeStorePatternComponent,
EventTriggerStateComponent,
EffectChainComponent,
Expand Down
Loading
Loading