From f9849a53e5b91c4deb29d5bb9689cdd151f8daf3 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 12 Jun 2026 06:18:39 +0000 Subject: [PATCH 1/2] fix: rules-of-hooks false positive on forwardRef/memo callbacks with non-PascalCase bindings Co-Authored-By: Aiden Bai --- ...ules-of-hooks-hoc-callback-binding-name.md | 5 ++ .../rules-of-hooks.regressions.test.ts | 79 +++++++++++++++++++ .../rules/react-builtins/rules-of-hooks.ts | 20 ++++- 3 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 .changeset/rules-of-hooks-hoc-callback-binding-name.md create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.regressions.test.ts diff --git a/.changeset/rules-of-hooks-hoc-callback-binding-name.md b/.changeset/rules-of-hooks-hoc-callback-binding-name.md new file mode 100644 index 000000000..3b60107f8 --- /dev/null +++ b/.changeset/rules-of-hooks-hoc-callback-binding-name.md @@ -0,0 +1,5 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +`rules-of-hooks` no longer reports false positives for hooks called inside a `forwardRef(...)` / `memo(...)` render callback whose binding name is not PascalCase (e.g. `const _Wrapped = forwardRef((props, ref) => { useHook(); ... })`). The render callback passed directly to React's HoCs is a component by construction, so the rule now treats it as one regardless of the variable name it lands on. Genuinely non-component functions like `const _helper = () => { useState(); }` still report. diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.regressions.test.ts new file mode 100644 index 000000000..0446e1707 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.regressions.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it } from "vite-plus/test"; +import { runRule } from "../../../test-utils/run-rule.js"; +import { rulesOfHooks } from "./rules-of-hooks.js"; + +const runTsx = (code: string) => runRule(rulesOfHooks, code, { filename: "fixture.tsx" }); + +describe("react-builtins/rules-of-hooks — regressions: HoC callbacks under non-PascalCase bindings", () => { + it("does not flag hooks in a forwardRef callback bound to an underscore-prefixed name", () => { + const result = runTsx(` + import { forwardRef, useState } from "react"; + const _Wrapped = forwardRef((props, ref) => { + const [value] = useState(0); + return
{value}
; + }); + export const Wrapped = _Wrapped; + `); + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag hooks in a memo callback bound to an underscore-prefixed name", () => { + const result = runTsx(` + import { memo, useState } from "react"; + const _Memoized = memo(function (props) { + const [value] = useState(0); + return {value}; + }); + export const Memoized = _Memoized; + `); + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag hooks in a React.forwardRef callback assigned to a lowercase binding", () => { + const result = runTsx(` + import * as React from "react"; + const _wrapped = React.forwardRef((props, ref) => { + React.useEffect(() => {}, []); + return
; + }); + export default _wrapped; + `); + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag hooks in a forwardRef callback inside memo(forwardRef(...))", () => { + const result = runTsx(` + import { memo, forwardRef, useState } from "react"; + const _Wrapped = memo( + forwardRef((props, ref) => { + const [value] = useState(0); + return
{value}
; + }), + ); + export const Wrapped = _Wrapped; + `); + expect(result.diagnostics).toEqual([]); + }); + + it("still flags hooks in a non-component underscore-prefixed function", () => { + const result = runTsx(` + import { useState } from "react"; + const _helper = () => { + const [value] = useState(0); + return value; + }; + `); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); + + it("still flags hooks in a named callback passed to an arbitrary non-React HoC", () => { + const result = runTsx(` + import { useState } from "react"; + const _wrapped = trackEvents(function _process() { + const [value] = useState(0); + return value; + }); + `); + expect(result.diagnostics.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.ts index d88249e48..4dc11efb9 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.ts @@ -422,6 +422,18 @@ const inferFunctionName = (functionNode: EsTreeNode): string | null => { return null; }; +// Mirrors upstream eslint-plugin-react-hooks: the render callback +// passed directly to `memo(...)` / `forwardRef(...)` IS a component +// by construction, regardless of what binding name it ends up under +// (`const _Wrapped = forwardRef((props, ref) => …)`). +const isReactHocCallbackArgument = (functionNode: EsTreeNode): boolean => { + const parent = functionNode.parent; + if (!parent || !isNodeOfType(parent, "CallExpression")) return false; + if (!parent.arguments.some((argument) => argument === functionNode)) return false; + const calleeName = getCallExpressionCalleeName(parent); + return calleeName !== null && REACT_HOC_NAMES.has(calleeName); +}; + const findEnclosingFunctionInfo = (node: EsTreeNode): FunctionInfo | null => { let current: EsTreeNode | null | undefined = node.parent; while (current) { @@ -430,14 +442,17 @@ const findEnclosingFunctionInfo = (node: EsTreeNode): FunctionInfo | null => { isNodeOfType(current, "FunctionExpression") || isNodeOfType(current, "ArrowFunctionExpression") ) { + const isHocCallback = isReactHocCallbackArgument(current); const resolvedName = inferFunctionName(current); const displayName = resolvedName ?? "anonymous"; return { node: current, name: displayName, - hasResolvedName: resolvedName !== null, + hasResolvedName: resolvedName !== null || isHocCallback, isAsync: Boolean(current.async), - isComponentOrHook: resolvedName === null ? false : isReactComponentOrHookName(displayName), + isComponentOrHook: + isHocCallback || + (resolvedName === null ? false : isReactComponentOrHookName(displayName)), }; } current = current.parent ?? null; @@ -553,6 +568,7 @@ const findEnclosingComponentOrHookFunction = (node: EsTreeNode): EsTreeNode | nu let current: EsTreeNode | null | undefined = node.parent; while (current) { if (isFunctionLike(current)) { + if (isReactHocCallbackArgument(current)) return current; const resolvedName = inferFunctionName(current); if (resolvedName !== null && isReactComponentOrHookName(resolvedName)) return current; } From 480f578d2cd89014e7bfbff6bc41ada94f3fe53d Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Fri, 12 Jun 2026 00:52:33 -0700 Subject: [PATCH 2/2] fix: restrict HoC-callback promotion to the render callback and share it with exhaustive-deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review fixes for the forwardRef/memo non-PascalCase binding FP fix: - Only the FIRST argument of memo/forwardRef is promoted to a component — memo's second argument is the props comparator, and promoting it silenced hooks-in-comparator violations that fired before this branch. - Restore `hasResolvedName`'s documented meaning (true iff a name was inferred) by checking `isComponentOrHook` before the anonymous walk-out instead of overloading the flag for HoC callbacks. - Extract `isReactHocCallbackArgument` into a shared util and apply it to exhaustive-deps' `findEnclosingComponentOrHookFunction`, which had the same FP: factory-scope captures inside a `const _Wrapped = forwardRef(...)` callback were reported as missing deps. - Strengthen the still-flags regression tests to assert the exact diagnostic message and cover the memo-comparator case. --- ...ules-of-hooks-hoc-callback-binding-name.md | 2 +- .../exhaustive-deps.regressions.test.ts | 26 ++++++++ .../rules/react-builtins/exhaustive-deps.ts | 2 + .../rules-of-hooks.regressions.test.ts | 28 +++++++- .../rules/react-builtins/rules-of-hooks.ts | 64 ++++++++----------- .../utils/is-react-hoc-callback-argument.ts | 31 +++++++++ 6 files changed, 112 insertions(+), 41 deletions(-) create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/utils/is-react-hoc-callback-argument.ts diff --git a/.changeset/rules-of-hooks-hoc-callback-binding-name.md b/.changeset/rules-of-hooks-hoc-callback-binding-name.md index 3b60107f8..6affa222f 100644 --- a/.changeset/rules-of-hooks-hoc-callback-binding-name.md +++ b/.changeset/rules-of-hooks-hoc-callback-binding-name.md @@ -2,4 +2,4 @@ "oxlint-plugin-react-doctor": patch --- -`rules-of-hooks` no longer reports false positives for hooks called inside a `forwardRef(...)` / `memo(...)` render callback whose binding name is not PascalCase (e.g. `const _Wrapped = forwardRef((props, ref) => { useHook(); ... })`). The render callback passed directly to React's HoCs is a component by construction, so the rule now treats it as one regardless of the variable name it lands on. Genuinely non-component functions like `const _helper = () => { useState(); }` still report. +`rules-of-hooks` and `exhaustive-deps` no longer report false positives for hooks called inside a `forwardRef(...)` / `memo(...)` render callback whose binding name is not PascalCase (e.g. `const _Wrapped = forwardRef((props, ref) => { useHook(); ... })`). The render callback passed as the first argument to React's HoCs is a component by construction, so both rules now treat it as one regardless of the variable name it lands on. Only the first argument is promoted — hooks inside `memo`'s second argument (the props comparator) still report, as do genuinely non-component functions like `const _helper = () => { useState(); }`. diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps.regressions.test.ts index 3c237a11d..6dd4c085b 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps.regressions.test.ts @@ -35,4 +35,30 @@ describe("react-builtins/exhaustive-deps — regressions", () => { const messages = result.diagnostics.map((diagnostic) => diagnostic.message).join("\n"); expect(messages).toContain("value"); }); + + // The render callback passed directly to forwardRef/memo is a + // component by construction, even under a non-PascalCase binding + // (`const _Wrapped = forwardRef(...)`). Without that promotion the + // component-scope boundary resolves to null and captures from an + // enclosing factory scope are wrongly reported as missing deps — + // they live outside the component, so they can't change between + // renders. + it("does not flag factory-scope captures inside a forwardRef callback under an underscore-prefixed binding", () => { + const code = ` + import { forwardRef, useEffect } from "react"; + const buildComponent = (logger) => { + const _Wrapped = forwardRef((props, ref) => { + useEffect(() => { + logger(props.value); + }, [props.value]); + return
; + }); + return _Wrapped; + }; + `; + const result = runRule(exhaustiveDeps, code, { filename: "fixture.tsx" }); + expect(result.parseErrors).toEqual([]); + const messages = result.diagnostics.map((diagnostic) => diagnostic.message).join("\n"); + expect(messages).not.toContain("logger"); + }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps.ts index fd2b3142f..25447c19e 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/exhaustive-deps.ts @@ -11,6 +11,7 @@ import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; import { getStaticTemplateLiteralValue } from "../../utils/get-static-template-literal-value.js"; import { isAstNode } from "../../utils/is-ast-node.js"; import { isReactComponentOrHookName } from "../../utils/is-react-component-or-hook-name.js"; +import { isReactHocCallbackArgument } from "../../utils/is-react-hoc-callback-argument.js"; import { isNodeOfType } from "../../utils/is-node-of-type.js"; import type { Rule } from "../../utils/rule.js"; import { REACT_HOC_NAMES } from "../../constants/react.js"; @@ -133,6 +134,7 @@ const findEnclosingComponentOrHookFunction = (node: EsTreeNode): EsTreeNode | nu isNodeOfType(current, "FunctionExpression") || isNodeOfType(current, "ArrowFunctionExpression") ) { + if (isReactHocCallbackArgument(current)) return current; const functionName = inferFunctionName(current); if (functionName && isReactComponentOrHookName(functionName)) return current; } diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.regressions.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.regressions.test.ts index 0446e1707..613199479 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.regressions.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.regressions.test.ts @@ -63,7 +63,10 @@ describe("react-builtins/rules-of-hooks — regressions: HoC callbacks under non return value; }; `); - expect(result.diagnostics.length).toBeGreaterThan(0); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0]?.message).toBe( + "`useState` runs inside `_helper`, which is not a component or Hook, so React cannot attach Hook state to a render.", + ); }); it("still flags hooks in a named callback passed to an arbitrary non-React HoC", () => { @@ -74,6 +77,27 @@ describe("react-builtins/rules-of-hooks — regressions: HoC callbacks under non return value; }); `); - expect(result.diagnostics.length).toBeGreaterThan(0); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0]?.message).toBe( + "`useState` runs inside `_process`, which is not a component or Hook, so React cannot attach Hook state to a render.", + ); + }); + + it("still flags hooks in a memo props comparator (second argument is not a render callback)", () => { + const result = runTsx(` + import { memo, useState } from "react"; + const _Memoized = memo( + (props) => {props.value}, + (previousProps, nextProps) => { + const [shouldSkip] = useState(false); + return shouldSkip; + }, + ); + export const Memoized = _Memoized; + `); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0]?.message).toBe( + "`useState` runs inside `_Memoized`, which is not a component or Hook, so React cannot attach Hook state to a render.", + ); }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.ts index 4dc11efb9..ca50a620f 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.ts @@ -7,6 +7,7 @@ import { isReactComponentOrHookName } from "../../utils/is-react-component-or-ho import { isReactHookName } from "../../utils/is-react-hook-name.js"; import { REACT_HOC_NAMES } from "../../constants/react.js"; import { isFunctionLike } from "../../utils/is-function-like.js"; +import { isReactHocCallbackArgument } from "../../utils/is-react-hoc-callback-argument.js"; import type { Rule } from "../../utils/rule.js"; // Port of `oxc_linter::rules::react::rules_of_hooks`. Enforces React's @@ -422,18 +423,6 @@ const inferFunctionName = (functionNode: EsTreeNode): string | null => { return null; }; -// Mirrors upstream eslint-plugin-react-hooks: the render callback -// passed directly to `memo(...)` / `forwardRef(...)` IS a component -// by construction, regardless of what binding name it ends up under -// (`const _Wrapped = forwardRef((props, ref) => …)`). -const isReactHocCallbackArgument = (functionNode: EsTreeNode): boolean => { - const parent = functionNode.parent; - if (!parent || !isNodeOfType(parent, "CallExpression")) return false; - if (!parent.arguments.some((argument) => argument === functionNode)) return false; - const calleeName = getCallExpressionCalleeName(parent); - return calleeName !== null && REACT_HOC_NAMES.has(calleeName); -}; - const findEnclosingFunctionInfo = (node: EsTreeNode): FunctionInfo | null => { let current: EsTreeNode | null | undefined = node.parent; while (current) { @@ -442,16 +431,15 @@ const findEnclosingFunctionInfo = (node: EsTreeNode): FunctionInfo | null => { isNodeOfType(current, "FunctionExpression") || isNodeOfType(current, "ArrowFunctionExpression") ) { - const isHocCallback = isReactHocCallbackArgument(current); const resolvedName = inferFunctionName(current); const displayName = resolvedName ?? "anonymous"; return { node: current, name: displayName, - hasResolvedName: resolvedName !== null || isHocCallback, + hasResolvedName: resolvedName !== null, isAsync: Boolean(current.async), isComponentOrHook: - isHocCallback || + isReactHocCallbackArgument(current) || (resolvedName === null ? false : isReactComponentOrHookName(displayName)), }; } @@ -706,32 +694,32 @@ export const rulesOfHooks = defineRule({ return; } - // For anonymous callbacks, look outward: if any enclosing - // function IS a component / custom hook, this nested anonymous - // callback can't legally call a hook (Rule of Hooks forbids - // hooks in nested callbacks even when the outer function is a - // component). When NO outer function is a component / hook, the - // callback's runtime context is unknown — skip to avoid false - // positives on generic callbacks (utility / event-handler - // factories). - if (!enclosing.hasResolvedName) { - let outerWalker: EsTreeNode | null = enclosing.node; - let outerIsComponentOrHook = false; - while (outerWalker) { - const outerInfo = findEnclosingFunctionInfo(outerWalker); - if (!outerInfo) break; - if (outerInfo.isComponentOrHook) { - outerIsComponentOrHook = true; - break; + if (!enclosing.isComponentOrHook) { + // For anonymous callbacks, look outward: if any enclosing + // function IS a component / custom hook, this nested anonymous + // callback can't legally call a hook (Rule of Hooks forbids + // hooks in nested callbacks even when the outer function is a + // component). When NO outer function is a component / hook, the + // callback's runtime context is unknown — skip to avoid false + // positives on generic callbacks (utility / event-handler + // factories). + if (!enclosing.hasResolvedName) { + let outerWalker: EsTreeNode | null = enclosing.node; + let outerIsComponentOrHook = false; + while (outerWalker) { + const outerInfo = findEnclosingFunctionInfo(outerWalker); + if (!outerInfo) break; + if (outerInfo.isComponentOrHook) { + outerIsComponentOrHook = true; + break; + } + outerWalker = outerInfo.node; } - outerWalker = outerInfo.node; + if (!outerIsComponentOrHook) return; + context.report({ node: node.callee, message: buildConditionalMessage(hookName) }); + return; } - if (!outerIsComponentOrHook) return; - context.report({ node: node.callee, message: buildConditionalMessage(hookName) }); - return; - } - if (!enclosing.isComponentOrHook) { context.report({ node: node.callee, message: buildNonComponentMessage(hookName, enclosing.name), diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-react-hoc-callback-argument.ts b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-react-hoc-callback-argument.ts new file mode 100644 index 000000000..911f6a9c0 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/utils/is-react-hoc-callback-argument.ts @@ -0,0 +1,31 @@ +import { REACT_HOC_NAMES } from "../constants/react.js"; +import type { EsTreeNode } from "./es-tree-node.js"; +import { isNodeOfType } from "./is-node-of-type.js"; + +const reactHocCalleeName = (callee: EsTreeNode): string | null => { + if (isNodeOfType(callee, "Identifier")) return callee.name; + if ( + isNodeOfType(callee, "MemberExpression") && + !callee.computed && + isNodeOfType(callee.object, "Identifier") && + callee.object.name === "React" && + isNodeOfType(callee.property, "Identifier") + ) { + return `React.${callee.property.name}`; + } + return null; +}; + +// Mirrors upstream eslint-plugin-react-hooks: the render callback +// passed as the FIRST argument to `memo(...)` / `forwardRef(...)` IS +// a component by construction, regardless of what binding name it +// ends up under (`const _Wrapped = forwardRef((props, ref) => …)`). +// Later arguments are not render callbacks (`memo`'s second argument +// is the props comparator), so they are never promoted. +export const isReactHocCallbackArgument = (functionNode: EsTreeNode): boolean => { + const parent = functionNode.parent; + if (!parent || !isNodeOfType(parent, "CallExpression")) return false; + if (parent.arguments[0] !== functionNode) return false; + const calleeName = reactHocCalleeName(parent.callee); + return calleeName !== null && REACT_HOC_NAMES.has(calleeName); +};