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..6affa222f --- /dev/null +++ b/.changeset/rules-of-hooks-hoc-callback-binding-name.md @@ -0,0 +1,5 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +`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 new file mode 100644 index 000000000..613199479 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/react-builtins/rules-of-hooks.regressions.test.ts @@ -0,0 +1,103 @@ +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).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", () => { + const result = runTsx(` + import { useState } from "react"; + const _wrapped = trackEvents(function _process() { + const [value] = useState(0); + return value; + }); + `); + 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 d88249e48..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 @@ -437,7 +438,9 @@ const findEnclosingFunctionInfo = (node: EsTreeNode): FunctionInfo | null => { name: displayName, hasResolvedName: resolvedName !== null, isAsync: Boolean(current.async), - isComponentOrHook: resolvedName === null ? false : isReactComponentOrHookName(displayName), + isComponentOrHook: + isReactHocCallbackArgument(current) || + (resolvedName === null ? false : isReactComponentOrHookName(displayName)), }; } current = current.parent ?? null; @@ -553,6 +556,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; } @@ -690,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); +};