diff --git a/packages/react-doctor/src/constants.ts b/packages/react-doctor/src/constants.ts index c4fdc0f..b08c955 100644 --- a/packages/react-doctor/src/constants.ts +++ b/packages/react-doctor/src/constants.ts @@ -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; diff --git a/packages/react-doctor/src/oxlint-config.ts b/packages/react-doctor/src/oxlint-config.ts index 384aa2f..962e664 100644 --- a/packages/react-doctor/src/oxlint-config.ts +++ b/packages/react-doctor/src/oxlint-config.ts @@ -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); @@ -241,6 +245,7 @@ export const GLOBAL_REACT_DOCTOR_RULES: Record = { "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", "react-doctor/prefer-useReducer": "warn", "react-doctor/prefer-use-sync-external-store": "warn", "react-doctor/rerender-lazy-state-init": "warn", @@ -382,6 +387,7 @@ const VERSION_GATED_RULE_IDS: ReadonlyMap = 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 = ( diff --git a/packages/react-doctor/src/plugin/constants.ts b/packages/react-doctor/src/plugin/constants.ts index 8536fb4..07c2239 100644 --- a/packages/react-doctor/src/plugin/constants.ts +++ b/packages/react-doctor/src/plugin/constants.ts @@ -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 diff --git a/packages/react-doctor/src/plugin/index.ts b/packages/react-doctor/src/plugin/index.ts index 7649599..7c667c4 100644 --- a/packages/react-doctor/src/plugin/index.ts +++ b/packages/react-doctor/src/plugin/index.ts @@ -190,6 +190,7 @@ import { noMutableInDeps, noPropCallbackInEffect, noSetStateInRender, + preferUseEffectEvent, preferUseReducer, preferUseSyncExternalStore, rerenderDependencies, @@ -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, 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 9b83903..df11069 100644 --- a/packages/react-doctor/src/plugin/rules/state-and-effects.ts +++ b/packages/react-doctor/src/plugin/rules/state-and-effects.ts @@ -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, @@ -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 => { + const functionTypedLocals = new Set(); + 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; + 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> = []; + + 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(); + } + }, + }; + }, +}; diff --git a/packages/react-doctor/src/utils/run-oxlint.ts b/packages/react-doctor/src/utils/run-oxlint.ts index 23444e1..d13b752 100644 --- a/packages/react-doctor/src/utils/run-oxlint.ts +++ b/packages/react-doctor/src/utils/run-oxlint.ts @@ -56,6 +56,7 @@ const RULE_CATEGORY_MAP: Record = { "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", @@ -262,6 +263,8 @@ const RULE_HELP_MAP: Record = { "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": 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 23ccede..54594b4 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 @@ -159,6 +159,15 @@ const MutableInDepsComponent = ({ token }: { token: string }) => { return
; }; +const PreferUseEffectEventComponent = ({ onSearch }: { onSearch: (q: string) => void }) => { + const [query, setQuery] = useState(""); + useEffect(() => { + const id = setTimeout(() => onSearch(query), 300); + return () => clearTimeout(id); + }, [query, onSearch]); + return setQuery(event.target.value)} />; +}; + declare const externalStore: { subscribe: (listener: () => void) => () => void; getSnapshot: () => number; @@ -258,6 +267,7 @@ export { EffectNeedsCleanupComponent, MirrorPropEffectComponent, MutableInDepsComponent, + PreferUseEffectEventComponent, SubscribeStorePatternComponent, EventTriggerStateComponent, EffectChainComponent, diff --git a/packages/react-doctor/tests/regressions/state-rules.test.ts b/packages/react-doctor/tests/regressions/state-rules.test.ts index 8d51929..276a7d7 100644 --- a/packages/react-doctor/tests/regressions/state-rules.test.ts +++ b/packages/react-doctor/tests/regressions/state-rules.test.ts @@ -15,6 +15,7 @@ afterAll(() => { const collectRuleHits = async ( projectDir: string, ruleId: string, + reactMajorVersion: number | null = null, ): Promise> => { const diagnostics = await runOxlint({ rootDirectory: projectDir, @@ -22,6 +23,7 @@ const collectRuleHits = async ( framework: "unknown", hasReactCompiler: false, hasTanStackQuery: false, + reactMajorVersion, }); return diagnostics .filter((diagnostic) => diagnostic.rule === ruleId) @@ -1961,3 +1963,352 @@ export const Stable = ({ onChange }: { onChange: () => void }) => { expect(hits).toHaveLength(0); }); }); + +describe("prefer-use-effect-event", () => { + it("flags the canonical setTimeout shape (Vercel `advanced-use-latest`)", async () => { + // https://react.dev/learn/separating-events-from-effects + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-settimeout", { + files: { + "src/SearchInput.tsx": `import { useEffect, useState } from "react"; + +export const SearchInput = ({ onSearch }: { onSearch: (q: string) => void }) => { + const [query, setQuery] = useState(""); + useEffect(() => { + const id = setTimeout(() => onSearch(query), 300); + return () => clearTimeout(id); + }, [query, onSearch]); + return setQuery(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("onSearch"); + expect(hits[0].message).toContain("setTimeout"); + }); + + it("flags an addEventListener handler that calls a prop callback", async () => { + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-add-listener", { + files: { + "src/Listener.tsx": `import { useEffect } from "react"; + +export const Listener = ({ onKey }: { onKey: (key: string) => void }) => { + useEffect(() => { + const handler = (event: KeyboardEvent) => onKey(event.key); + window.addEventListener("keydown", handler); + return () => window.removeEventListener("keydown", handler); + }, [onKey]); + return ; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + // Single dep array — needs >= 2 deps for the rule to fire. + expect(hits).toHaveLength(0); + }); + + it("flags an addEventListener handler with multiple deps including the callback", async () => { + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-multi-deps", { + files: { + "src/Listener.tsx": `import { useEffect, useState } from "react"; + +export const Listener = ({ onKey }: { onKey: (key: string, prefix: string) => void }) => { + const [prefix, setPrefix] = useState(""); + useEffect(() => { + const handler = (event: KeyboardEvent) => onKey(event.key, prefix); + window.addEventListener("keydown", handler); + return () => window.removeEventListener("keydown", handler); + }, [prefix, onKey]); + return setPrefix(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("onKey"); + expect(hits[0].message).toContain("addEventListener"); + }); + + it("flags a store.subscribe handler that calls a prop callback", async () => { + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-subscribe", { + files: { + "src/Logger.tsx": `import { useEffect, useState } from "react"; + +declare const store: { subscribe: (handler: () => void) => () => void }; + +export const Logger = ({ onChange }: { onChange: (value: number) => void }) => { + const [value, setValue] = useState(0); + useEffect(() => { + const unsubscribe = store.subscribe(() => onChange(value)); + return unsubscribe; + }, [value, onChange]); + return ; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("onChange"); + expect(hits[0].message).toContain("subscribe"); + }); + + it("does NOT flag a callback that is read at the effect's top level (true reactive read)", async () => { + // The article is explicit: only non-reactive reads should move into + // useEffectEvent. If the callback is part of the start-sync expression + // itself, it really should be in deps. + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-top-level", { + files: { + "src/Mount.tsx": `import { useEffect, useState } from "react"; + +export const Mount = ({ onMount }: { onMount: (q: string) => void }) => { + const [query, setQuery] = useState(""); + useEffect(() => { + onMount(query); + }, [query, onMount]); + return setQuery(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + expect(hits).toHaveLength(0); + }); + + it("does NOT flag when the dep is not function-typed (state, plain identifier)", async () => { + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-non-fn-dep", { + files: { + "src/Counter.tsx": `import { useEffect, useState } from "react"; + +declare const log: (count: number) => void; + +export const Counter = () => { + const [count, setCount] = useState(0); + const [base, setBase] = useState(0); + useEffect(() => { + const id = setTimeout(() => log(count + base), 100); + return () => clearTimeout(id); + }, [count, base]); + return ; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + expect(hits).toHaveLength(0); + }); + + it("does NOT flag when the dep array has fewer than 2 elements (single-dep effect doesn't benefit)", async () => { + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-single-dep", { + files: { + "src/Single.tsx": `import { useEffect } from "react"; + +export const Single = ({ onTick }: { onTick: () => void }) => { + useEffect(() => { + const id = setInterval(() => onTick(), 1000); + return () => clearInterval(id); + }, [onTick]); + return ; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + expect(hits).toHaveLength(0); + }); + + it("flags a `useCallback`-bound local that is only invoked from a sub-handler", async () => { + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-usecallback", { + files: { + "src/Spy.tsx": `import { useCallback, useEffect, useState } from "react"; + +declare const audit: (event: string) => void; + +export const Spy = ({ tag }: { tag: string }) => { + const [count, setCount] = useState(0); + const log = useCallback(() => audit(tag), [tag]); + useEffect(() => { + const id = setInterval(() => log(), 1000); + return () => clearInterval(id); + }, [count, log]); + return {count}; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("log"); + expect(hits[0].message).toContain("setInterval"); + }); + + it("fires when reactMajorVersion is explicitly 19", async () => { + // useEffectEvent landed in React 19. The rule should still fire when + // the project is detected as React 19 — same diagnostic as the default + // (null) path. + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-react-19", { + reactVersion: "^19.0.0", + files: { + "src/SearchInput.tsx": `import { useEffect, useState } from "react"; + +export const SearchInput = ({ onSearch }: { onSearch: (q: string) => void }) => { + const [query, setQuery] = useState(""); + useEffect(() => { + const id = setTimeout(() => onSearch(query), 300); + return () => clearTimeout(id); + }, [query, onSearch]); + return setQuery(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event", 19); + expect(hits).toHaveLength(1); + expect(hits[0].message).toContain("onSearch"); + }); + + it("does NOT fire when reactMajorVersion is below the useEffectEvent threshold (React 18)", async () => { + // Recommending useEffectEvent on React 18 produces noisy diagnostics + // for users who don't have the API. The rule is gated to React >= 19. + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-react-18", { + reactVersion: "^18.3.0", + files: { + "src/SearchInput.tsx": `import { useEffect, useState } from "react"; + +export const SearchInput = ({ onSearch }: { onSearch: (q: string) => void }) => { + const [query, setQuery] = useState(""); + useEffect(() => { + const id = setTimeout(() => onSearch(query), 300); + return () => clearTimeout(id); + }, [query, onSearch]); + return setQuery(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event", 18); + expect(hits).toHaveLength(0); + }); + + it("does NOT fire when reactMajorVersion is React 17", async () => { + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-react-17", { + reactVersion: "^17.0.0", + files: { + "src/SearchInput.tsx": `import { useEffect, useState } from "react"; + +export const SearchInput = ({ onSearch }: { onSearch: (q: string) => void }) => { + const [query, setQuery] = useState(""); + useEffect(() => { + const id = setTimeout(() => onSearch(query), 300); + return () => clearTimeout(id); + }, [query, onSearch]); + return setQuery(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event", 17); + expect(hits).toHaveLength(0); + }); + + it("does NOT flag a useEffect inside a nested helper that closes over an OUTER component's prop", async () => { + // The empty-frame barrier prevents the inner non-component helper + // from inheriting the outer component's prop set. `value` is closed + // over by Inner via lexical scope, but it is NOT a prop of Inner — + // so the rule must not fire there. + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-nested-helper", { + files: { + "src/Outer.tsx": `import { useEffect, useState } from "react"; + +export const Outer = ({ value }: { value: (q: string) => void }) => { + const [query, setQuery] = useState(""); + function inner() { + useEffect(() => { + const id = setTimeout(() => value(query), 300); + return () => clearTimeout(id); + }, [query, value]); + } + inner(); + return setQuery(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + expect(hits).toHaveLength(0); + }); + + it("does NOT flag a scalar destructured prop only read inside a sub-handler (Bugbot #162)", async () => { + // Regression: previously every destructured prop satisfied the + // function-typed gate. A component like \`({ onSearch, prefix })\` + // would get \`prefix\` (a string) flagged with a 'wrap in + // useEffectEvent' message — semantically wrong for non-functions. + // Now only \`on[A-Z]\`-shaped prop names pass; \`prefix\` does not. + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-scalar-prop", { + files: { + "src/SearchInput.tsx": `import { useEffect, useState } from "react"; + +export const SearchInput = ({ + onSearch, + prefix, +}: { + onSearch: (query: string) => void; + prefix: string; +}) => { + const [query, setQuery] = useState(""); + useEffect(() => { + const id = setTimeout(() => onSearch(\`\${prefix}\${query}\`), 300); + return () => clearTimeout(id); + }, [query, prefix, onSearch]); + return setQuery(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event"); + // \`onSearch\` (an on*-named prop) IS validly flagged. + // \`prefix\` (a scalar string) MUST NOT be flagged. + expect(hits.length).toBe(1); + expect(hits[0].message).toContain("onSearch"); + expect(hits[0].message).not.toContain("prefix"); + }); + + it("fires when reactMajorVersion is unknown (null) so we never silently swallow real findings", async () => { + // Matches the existing convention — null version detection keeps every + // rule enabled. See `filterRulesByReactMajor` in oxlint-config.ts. + const projectDir = setupReactProject(tempRoot, "prefer-use-effect-event-unknown-version", { + files: { + "src/SearchInput.tsx": `import { useEffect, useState } from "react"; + +export const SearchInput = ({ onSearch }: { onSearch: (q: string) => void }) => { + const [query, setQuery] = useState(""); + useEffect(() => { + const id = setTimeout(() => onSearch(query), 300); + return () => clearTimeout(id); + }, [query, onSearch]); + return setQuery(event.target.value)} />; +}; +`, + }, + }); + + const hits = await collectRuleHits(projectDir, "prefer-use-effect-event", null); + expect(hits).toHaveLength(1); + }); +}); diff --git a/packages/react-doctor/tests/run-oxlint.test.ts b/packages/react-doctor/tests/run-oxlint.test.ts index 659abfa..92143dc 100644 --- a/packages/react-doctor/tests/run-oxlint.test.ts +++ b/packages/react-doctor/tests/run-oxlint.test.ts @@ -162,6 +162,12 @@ describe("runOxlint", () => { ruleSource: "rules/state-and-effects.ts", severity: "warning", }, + "prefer-use-effect-event": { + fixture: "state-issues.tsx", + ruleSource: "rules/state-and-effects.ts", + severity: "warning", + category: "State & Effects", + }, "prefer-useReducer": { fixture: "state-issues.tsx", ruleSource: "rules/state-and-effects.ts",