From 62475c7ed456540369a6c0daef756f17ba0dd0b1 Mon Sep 17 00:00:00 2001 From: Nisarg Patel Date: Fri, 5 Jun 2026 16:38:24 -0700 Subject: [PATCH 1/3] feat(rules): add prefer-existing-hook-library Catches top-level custom hooks whose names match a hook already shipped by react-use or usehooks-ts (useDebounce, useLocalStorage, useOnClickOutside, useToggle, usePrevious, useEventListener, useInterval, useMount, useUpdateEffect, ... ~95 names). Hand-rolled versions commonly miss SSR safety, cleanup races, stale closures on identity-unstable callbacks, and Strict-Mode double-fire semantics that the library hooks already handle. Detection is name-based + scope-aware: only module-level FunctionDeclaration / VariableDeclarator whose body actually contains React hook calls are flagged. Hooks defined inside another component or hook, utilities that happen to start with "use" but never call a hook, pure re-exports, and single-statement delegation wrappers (including renamed-import facades like `import { useDebounce as upstream } from "react-use"; export const useDebounce = (v) => upstream(v, 500)`) are skipped. Tagged "test-noise" so fixture / story / test files auto-skip. Ambiguous names that clash with React / routing / animation libraries (useLocation, useEvent, useEventCallback, useSearchParams, useNavigation, useRouter, useSpring, useHash) are intentionally excluded from the match list so the rule stays high-precision. 24 adversarial tests cover arrow-expression bodies, parenthesized hook-call bodies, TS `as` wrappers, default exports, ambient declarations, class methods, identifier re-bindings, and renamed-import facades. Co-authored-by: Cursor --- .changeset/prefer-existing-hook-library.md | 9 + .../src/plugin/constants/hook-libraries.ts | 198 ++++++++ .../src/plugin/rule-registry.ts | 12 + .../prefer-existing-hook-library.test.ts | 450 ++++++++++++++++++ .../prefer-existing-hook-library.ts | 232 +++++++++ 5 files changed, 901 insertions(+) create mode 100644 .changeset/prefer-existing-hook-library.md create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.test.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.ts diff --git a/.changeset/prefer-existing-hook-library.md b/.changeset/prefer-existing-hook-library.md new file mode 100644 index 000000000..0bfadcc60 --- /dev/null +++ b/.changeset/prefer-existing-hook-library.md @@ -0,0 +1,9 @@ +--- +"oxlint-plugin-react-doctor": patch +--- + +Add `prefer-existing-hook-library` (Maintainability, warn): catches top-level custom hooks whose names match a hook already shipped by `react-use` or `usehooks-ts` (`useDebounce`, `useLocalStorage`, `useOnClickOutside`, `useToggle`, `usePrevious`, `useEventListener`, `useInterval`, `useMount`, `useUpdateEffect`, … ~95 names in total). Hand-rolled versions of these hooks commonly miss SSR safety, cleanup races, stale closures on identity-unstable callbacks, and Strict-Mode double-fire semantics that the library hooks already handle. + +- Detection is **name-based + scope-aware**: only module-level `FunctionDeclaration` / `VariableDeclarator` whose body actually contains React hook calls are flagged. Hooks defined inside another component or hook, utilities that happen to start with `use` but never call a hook, pure re-exports (`export { useDebounce } from "..."`), and single-statement delegation wrappers (including renamed-import facades like `import { useDebounce as upstream } from "react-use"; export const useDebounce = (v) => upstream(v, 500)`) are skipped. +- Ambiguous names that clash with React, routing libraries, or animation libraries (`useLocation`, `useEvent`, `useEventCallback`, `useSearchParams`, `useNavigation`, `useRouter`, `useSpring`, `useHash`) are intentionally excluded from the match list so the rule stays high-precision. +- Tagged `test-noise` so test / fixture / story files auto-skip the check. diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts new file mode 100644 index 000000000..8dd28acee --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts @@ -0,0 +1,198 @@ +// Canonical hook names shipped by the two popular utility-hook libraries +// `react-use` (streamich) and `usehooks-ts` (juliencrn). Source of truth +// for `prefer-existing-hook-library`, which flags top-level custom hooks +// whose names match this map so users can adopt the library version +// instead of hand-rolling one (hand-rolled versions commonly miss +// SSR safety, cleanup races, stale closures on identity-unstable +// callbacks, Strict-Mode double-fire, and cross-tab sync). +// +// Curation rules for this map: +// - Only include names that are STRONGLY conventional. Everyone knows +// what `useDebounce` does; the FP risk of a same-name custom hook +// doing something completely different is low. +// - EXCLUDE names that clash with framework/library hooks of the same +// name (e.g. `useLocation` / `useNavigation` / `useSearchParams` from +// react-router, `useEvent` / `useEventCallback` from React's own +// experimental APIs, `useSpring` from react-spring). Detection is +// name-only, so an ambiguous match is a false positive. +// - List BOTH libraries when both ship the same name so the diagnostic +// can recommend the one the user already has installed. + +export interface HookLibraryAvailability { + readonly reactUse: boolean; + readonly usehooksTs: boolean; +} + +// HACK: `useEvent` (react-use's plain event-subscribe hook) is intentionally +// OMITTED — React 19 also ships an experimental `useEvent` from `react`, +// and several router libraries reuse the name. Flagging on this name would +// false-positive on legitimate non-react-use code. Use the unambiguous +// `useEventListener` (usehooks-ts) entry instead. +// +// HACK: `useLocation`, `useNavigation`, `useSearchParams`, `useSearchParam`, +// `useHash`, `useRouter`, `useParams`, `usePathname` are omitted for the +// same reason: every routing library (react-router, Next, TanStack Router, +// remix, expo-router) ships hooks with these names. +// +// HACK: `useEventCallback` is omitted — both React-experimental and +// usehooks-ts use the name with different semantics. We don't want to +// recommend a swap on a hook that may be the React-native one. +export const HOOK_LIBRARY_MAP: ReadonlyMap = + new Map([ + // Side-effects / timing + ["useDebounce", { reactUse: true, usehooksTs: false }], + ["useDebounceValue", { reactUse: false, usehooksTs: true }], + ["useDebounceCallback", { reactUse: false, usehooksTs: true }], + ["useThrottle", { reactUse: true, usehooksTs: false }], + ["useThrottleFn", { reactUse: true, usehooksTs: false }], + ["useInterval", { reactUse: true, usehooksTs: true }], + ["useHarmonicIntervalFn", { reactUse: true, usehooksTs: false }], + ["useTimeout", { reactUse: true, usehooksTs: true }], + ["useTimeoutFn", { reactUse: true, usehooksTs: false }], + ["useCountdown", { reactUse: false, usehooksTs: true }], + + // Storage + ["useLocalStorage", { reactUse: true, usehooksTs: true }], + ["useSessionStorage", { reactUse: true, usehooksTs: true }], + ["useReadLocalStorage", { reactUse: false, usehooksTs: true }], + ["useCookie", { reactUse: true, usehooksTs: false }], + + // Lifecycle helpers + ["useMount", { reactUse: true, usehooksTs: false }], + ["useUnmount", { reactUse: true, usehooksTs: true }], + ["useUpdateEffect", { reactUse: true, usehooksTs: true }], + ["useEffectOnce", { reactUse: true, usehooksTs: true }], + ["useIsomorphicLayoutEffect", { reactUse: true, usehooksTs: true }], + ["useDeepCompareEffect", { reactUse: true, usehooksTs: false }], + ["useShallowCompareEffect", { reactUse: true, usehooksTs: false }], + ["useCustomCompareEffect", { reactUse: true, usehooksTs: false }], + ["useLifecycles", { reactUse: true, usehooksTs: false }], + ["useLogger", { reactUse: true, usehooksTs: false }], + + // Mount / first-render trackers + ["useIsMounted", { reactUse: false, usehooksTs: true }], + ["useMountedState", { reactUse: true, usehooksTs: false }], + ["useFirstMountState", { reactUse: true, usehooksTs: false }], + ["useIsFirstRender", { reactUse: false, usehooksTs: true }], + ["useIsClient", { reactUse: false, usehooksTs: true }], + ["useSsr", { reactUse: false, usehooksTs: true }], + + // Previous / latest value + ["usePrevious", { reactUse: true, usehooksTs: false }], + ["usePreviousDistinct", { reactUse: true, usehooksTs: false }], + ["useLatest", { reactUse: true, usehooksTs: false }], + + // Boolean / counter / step state + ["useToggle", { reactUse: true, usehooksTs: true }], + ["useBoolean", { reactUse: true, usehooksTs: true }], + ["useCounter", { reactUse: true, usehooksTs: true }], + ["useNumber", { reactUse: true, usehooksTs: false }], + ["useStep", { reactUse: false, usehooksTs: true }], + + // Collection state + ["useList", { reactUse: true, usehooksTs: false }], + ["useMap", { reactUse: true, usehooksTs: true }], + ["useSet", { reactUse: true, usehooksTs: false }], + ["useQueue", { reactUse: true, usehooksTs: false }], + ["useStateList", { reactUse: true, usehooksTs: false }], + ["useStateWithHistory", { reactUse: true, usehooksTs: false }], + + // Setter-shaped state + ["useSetState", { reactUse: true, usehooksTs: false }], + ["useGetSet", { reactUse: true, usehooksTs: false }], + ["useGetSetState", { reactUse: true, usehooksTs: false }], + ["useDefault", { reactUse: true, usehooksTs: false }], + ["useMediatedState", { reactUse: true, usehooksTs: false }], + + // Render / async + ["useRendersCount", { reactUse: true, usehooksTs: false }], + ["useUpdate", { reactUse: true, usehooksTs: false }], + ["useAsync", { reactUse: true, usehooksTs: false }], + ["useAsyncFn", { reactUse: true, usehooksTs: false }], + ["useAsyncRetry", { reactUse: true, usehooksTs: false }], + ["usePromise", { reactUse: true, usehooksTs: false }], + ["useObservable", { reactUse: true, usehooksTs: false }], + ["useMethods", { reactUse: true, usehooksTs: false }], + + // DOM observation + ["useClickAway", { reactUse: true, usehooksTs: false }], + ["useOnClickOutside", { reactUse: false, usehooksTs: true }], + ["useClickAnyWhere", { reactUse: false, usehooksTs: true }], + ["useEventListener", { reactUse: false, usehooksTs: true }], + ["useHover", { reactUse: true, usehooksTs: true }], + ["useHoverDirty", { reactUse: true, usehooksTs: false }], + ["useIntersection", { reactUse: true, usehooksTs: false }], + ["useIntersectionObserver", { reactUse: false, usehooksTs: true }], + ["useResizeObserver", { reactUse: false, usehooksTs: true }], + ["useMeasure", { reactUse: true, usehooksTs: false }], + ["useSize", { reactUse: true, usehooksTs: false }], + ["useLongPress", { reactUse: true, usehooksTs: false }], + ["useScratch", { reactUse: true, usehooksTs: false }], + ["useScroll", { reactUse: true, usehooksTs: false }], + ["useScrolling", { reactUse: true, usehooksTs: false }], + ["useWindowScroll", { reactUse: true, usehooksTs: false }], + ["useWindowSize", { reactUse: true, usehooksTs: true }], + ["usePageLeave", { reactUse: true, usehooksTs: false }], + ["useScrollbarWidth", { reactUse: true, usehooksTs: false }], + ["usePinchZoom", { reactUse: true, usehooksTs: false }], + ["useScrollLock", { reactUse: false, usehooksTs: true }], + ["useLockBodyScroll", { reactUse: true, usehooksTs: false }], + ["useLockedBody", { reactUse: false, usehooksTs: true }], + + // Device / browser state + ["useBattery", { reactUse: true, usehooksTs: false }], + ["useGeolocation", { reactUse: true, usehooksTs: false }], + ["useMedia", { reactUse: true, usehooksTs: false }], + ["useMediaQuery", { reactUse: false, usehooksTs: true }], + ["useMediaDevices", { reactUse: true, usehooksTs: false }], + ["useNetworkState", { reactUse: true, usehooksTs: false }], + ["useOrientation", { reactUse: true, usehooksTs: false }], + ["useScreen", { reactUse: false, usehooksTs: true }], + ["useMotion", { reactUse: true, usehooksTs: false }], + ["useMouse", { reactUse: true, usehooksTs: false }], + ["useMouseHovered", { reactUse: true, usehooksTs: false }], + ["useMouseWheel", { reactUse: true, usehooksTs: false }], + ["useIdle", { reactUse: true, usehooksTs: false }], + ["usePermission", { reactUse: true, usehooksTs: false }], + ["useStartTyping", { reactUse: true, usehooksTs: false }], + ["useBeforeUnload", { reactUse: true, usehooksTs: false }], + ["useKey", { reactUse: true, usehooksTs: false }], + ["useKeyPress", { reactUse: true, usehooksTs: false }], + ["useKeyPressEvent", { reactUse: true, usehooksTs: false }], + ["useKeyboardJs", { reactUse: true, usehooksTs: false }], + + // Page / document chrome + ["useTitle", { reactUse: true, usehooksTs: false }], + ["useDocumentTitle", { reactUse: false, usehooksTs: true }], + ["useFavicon", { reactUse: true, usehooksTs: false }], + ["useDarkMode", { reactUse: false, usehooksTs: true }], + ["useTernaryDarkMode", { reactUse: false, usehooksTs: true }], + + // Clipboard / loading / scripts + ["useCopyToClipboard", { reactUse: true, usehooksTs: true }], + ["useScript", { reactUse: false, usehooksTs: true }], + + // Media UI + ["useAudio", { reactUse: true, usehooksTs: false }], + ["useVideo", { reactUse: true, usehooksTs: false }], + ["useFullscreen", { reactUse: true, usehooksTs: false }], + ["useSlider", { reactUse: true, usehooksTs: false }], + ["useSpeech", { reactUse: true, usehooksTs: false }], + ["useVibrate", { reactUse: true, usehooksTs: false }], + + // Animation / RAF + ["useRaf", { reactUse: true, usehooksTs: false }], + ["useRafLoop", { reactUse: true, usehooksTs: false }], + ["useRafState", { reactUse: true, usehooksTs: false }], + ["useTween", { reactUse: true, usehooksTs: false }], + + // Refs + ["useEnsuredForwardedRef", { reactUse: true, usehooksTs: false }], + + // Drop-zones + ["useDrop", { reactUse: true, usehooksTs: false }], + ["useDropArea", { reactUse: true, usehooksTs: false }], + + // Misc errors + ["useError", { reactUse: true, usehooksTs: false }], + ]); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts index 083f7311f..c4c204527 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts @@ -235,6 +235,7 @@ import { preactPreferOndblclick } from "./rules/preact/preact-prefer-ondblclick. import { preactPreferOninput } from "./rules/preact/preact-prefer-oninput.js"; import { preferDynamicImport } from "./rules/bundle-size/prefer-dynamic-import.js"; import { preferEs6Class } from "./rules/react-builtins/prefer-es6-class.js"; +import { preferExistingHookLibrary } from "./rules/architecture/prefer-existing-hook-library.js"; import { preferFunctionComponent } from "./rules/react-builtins/prefer-function-component.js"; import { preferHtmlDialog } from "./rules/a11y/prefer-html-dialog.js"; import { preferModuleScopePureFunction } from "./rules/architecture/prefer-module-scope-pure-function.js"; @@ -2843,6 +2844,17 @@ export const reactDoctorRules = [ category: "Maintainability", }, }, + { + key: "react-doctor/prefer-existing-hook-library", + id: "prefer-existing-hook-library", + source: "react-doctor", + originallyExternal: false, + rule: { + ...preferExistingHookLibrary, + framework: "global", + category: "Maintainability", + }, + }, { key: "react-doctor/prefer-function-component", id: "prefer-function-component", diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.test.ts new file mode 100644 index 000000000..d4ae1da77 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.test.ts @@ -0,0 +1,450 @@ +import { describe, expect, it } from "vite-plus/test"; +import { runRule } from "../../../test-utils/run-rule.js"; +import { preferExistingHookLibrary } from "./prefer-existing-hook-library.js"; + +describe("prefer-existing-hook-library", () => { + describe("flags top-level reimplementations", () => { + it("flags a `useDebounce` function declaration", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect, useState } from "react"; + + function useDebounce(value, delay) { + const [debounced, setDebounced] = useState(value); + useEffect(() => { + const handle = setTimeout(() => setDebounced(value), delay); + return () => clearTimeout(handle); + }, [value, delay]); + return debounced; + } + ` + ); + + expect(result.parseErrors).toEqual([]); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useDebounce"); + expect(result.diagnostics[0].message).toContain("react-use"); + }); + + it("flags a `useLocalStorage` arrow function const", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useCallback, useState } from "react"; + + const useLocalStorage = (key, initial) => { + const [value, setValue] = useState(() => { + const raw = localStorage.getItem(key); + return raw === null ? initial : JSON.parse(raw); + }); + const setAndStore = useCallback((next) => { + setValue(next); + localStorage.setItem(key, JSON.stringify(next)); + }, [key]); + return [value, setAndStore]; + }; + ` + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useLocalStorage"); + expect(result.diagnostics[0].message).toContain("react-use"); + expect(result.diagnostics[0].message).toContain("usehooks-ts"); + }); + + it("flags an exported `useOnClickOutside` declaration", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect } from "react"; + + export function useOnClickOutside(ref, handler) { + useEffect(() => { + const listener = (event) => { + if (!ref.current || ref.current.contains(event.target)) return; + handler(event); + }; + document.addEventListener("mousedown", listener); + return () => document.removeEventListener("mousedown", listener); + }, [ref, handler]); + } + ` + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useOnClickOutside"); + expect(result.diagnostics[0].message).toContain("usehooks-ts"); + }); + + it("flags an `export const useToggle =` arrow", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useCallback, useState } from "react"; + + export const useToggle = (initial = false) => { + const [value, setValue] = useState(initial); + const toggle = useCallback(() => setValue((current) => !current), []); + return [value, toggle]; + }; + ` + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useToggle"); + }); + + it("flags a TS-typed `useThrottle` const", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect, useRef, useState } from "react"; + + export const useThrottle = (value: T, limit: number): T => { + const [throttled, setThrottled] = useState(value); + const lastRan = useRef(Date.now()); + useEffect(() => { + const handle = setTimeout(() => { + if (Date.now() - lastRan.current >= limit) { + setThrottled(value); + lastRan.current = Date.now(); + } + }, limit - (Date.now() - lastRan.current)); + return () => clearTimeout(handle); + }, [value, limit]); + return throttled; + }; + ` + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useThrottle"); + }); + + it("flags a `usePrevious` hook with a single useRef call", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect, useRef } from "react"; + + function usePrevious(value) { + const ref = useRef(); + useEffect(() => { + ref.current = value; + }); + return ref.current; + } + ` + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("usePrevious"); + expect(result.diagnostics[0].message).toContain("react-use"); + }); + + it("flags an arrow with expression body that's a direct hook call", () => { + // Regression: without the body-as-CallExpression check in + // findReactHookCallInOwnBody, this one-liner reimplementation of + // `useMount` slipped past while the parenthesized variant + // `(cb) => (useEffect(cb, []))` was correctly caught — inconsistent. + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect } from "react"; + + export const useMount = (cb) => useEffect(cb, []); + ` + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useMount"); + }); + + it("flags an arrow with parenthesized hook-call body (consistency)", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect } from "react"; + + export const useMount = (cb) => (useEffect(cb, [])); + ` + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useMount"); + }); + + it("flags an `export default function useDebounce` declaration", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect, useState } from "react"; + + export default function useDebounce(value, delay) { + const [debounced, setDebounced] = useState(value); + useEffect(() => { + const handle = setTimeout(() => setDebounced(value), delay); + return () => clearTimeout(handle); + }, [value, delay]); + return debounced; + } + ` + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useDebounce"); + }); + }); + + describe("does not flag legitimate code", () => { + it("does not flag a hook with no matching library name", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useState } from "react"; + + function useMyCustomBusinessLogic() { + const [value, setValue] = useState(0); + return [value, setValue]; + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag a same-named function with NO React hook calls", () => { + const result = runRule( + preferExistingHookLibrary, + ` + export function useDebounce(value, delay) { + let last = 0; + return () => { + const now = Date.now(); + if (now - last < delay) return; + last = now; + return value; + }; + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag a hook DEFINED INSIDE another component", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect, useState } from "react"; + + export function MyApp() { + function useDebounce(value, delay) { + const [debounced, setDebounced] = useState(value); + useEffect(() => { + const handle = setTimeout(() => setDebounced(value), delay); + return () => clearTimeout(handle); + }, [value, delay]); + return debounced; + } + return null; + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag a delegation wrapper that returns a same-named hook", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useDebounce as useDebounceUpstream } from "react-use"; + + export function useDebounce(value) { + return useDebounceUpstream(value, 500); + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag an arrow-body delegation wrapper", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useDebounce as upstream } from "react-use"; + + export const useDebounce = (value) => useDebounce(value, 500); + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag a pure re-export", () => { + const result = runRule( + preferExistingHookLibrary, + ` + export { useDebounce } from "react-use"; + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag a hook whose name only ambiguously matches React/router APIs", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect, useState } from "react"; + + export function useLocation() { + const [pathname, setPathname] = useState(window.location.pathname); + useEffect(() => { + const onPop = () => setPathname(window.location.pathname); + window.addEventListener("popstate", onPop); + return () => window.removeEventListener("popstate", onPop); + }, []); + return { pathname }; + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag a PascalCase same-name binding", () => { + const result = runRule( + preferExistingHookLibrary, + ` + export class UseDebounce { + run(value) { + return value; + } + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not descend into nested function bodies when checking own-body hook calls", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect } from "react"; + + export const useEventListener = (target, type, callback) => { + function attach() { + const handler = () => useState(0); + target.addEventListener(type, handler); + return () => target.removeEventListener(type, handler); + } + attach(); + }; + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag a hook defined inside another hook", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect, useState } from "react"; + + export function useResetOnNavigate() { + function useToggle() { + const [value, setValue] = useState(false); + useEffect(() => {}, []); + return [value, setValue]; + } + return useToggle(); + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not double-report multiple shadowed declarations with the same name", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect, useState } from "react"; + + function useDebounce(value) { + const [debounced, setDebounced] = useState(value); + useEffect(() => {}, [value]); + return debounced; + } + ` + ); + + expect(result.diagnostics).toHaveLength(1); + }); + + it("does not flag a class method named like a library hook", () => { + const result = runRule( + preferExistingHookLibrary, + ` + export class HookFactory { + useDebounce(value, delay) { + return value; + } + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag a TypeScript ambient `declare function`", () => { + const result = runRule( + preferExistingHookLibrary, + ` + declare function useDebounce(value: T, delay: number): T; + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag an anonymous default-exported function", () => { + // No id binding to compare against the hook-name map. + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect, useState } from "react"; + + export default function (value) { + const [v] = useState(value); + useEffect(() => {}, []); + return v; + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + + it("does not flag a re-binding via identifier assignment", () => { + // VariableDeclarator.init is an Identifier, not a function — the + // visitor early-returns before the hook-map lookup. + const result = runRule( + preferExistingHookLibrary, + ` + import { useDebounce as upstream } from "react-use"; + + export const useDebounce = upstream; + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + }); +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.ts new file mode 100644 index 000000000..86b3c54be --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.ts @@ -0,0 +1,232 @@ +import { + HOOK_LIBRARY_MAP, + type HookLibraryAvailability, +} from "../../constants/hook-libraries.js"; +import { defineRule } from "../../utils/define-rule.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; +import { getCalleeName } from "../../utils/get-callee-name.js"; +import { getImportedNameFromModule } from "../../utils/find-import-source-for-name.js"; +import { isNodeOfType } from "../../utils/is-node-of-type.js"; +import { isReactHookName } from "../../utils/is-react-hook-name.js"; +import { stripParenExpression } from "../../utils/strip-paren-expression.js"; +import { walkAst } from "../../utils/walk-ast.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +const HOOK_LIBRARY_MODULE_SOURCES = ["react-use", "usehooks-ts"] as const; + +// True when `node` is at the file's module/top level — i.e. its parent is +// the `Program` root, optionally through one `ExportNamedDeclaration` or +// `ExportDefaultDeclaration` wrapper. Nested hooks (defined inside a +// component or another hook) are out of v1 scope because legitimate +// factory-style nesting is common (`useResetOnNavigate` calling +// `usePrevious` internally) and the same-name match would false-fire. +const isAtModuleScope = (node: EsTreeNode): boolean => { + const parent = node.parent; + if (!parent) return false; + if (isNodeOfType(parent, "Program")) return true; + if ( + isNodeOfType(parent, "ExportNamedDeclaration") || + isNodeOfType(parent, "ExportDefaultDeclaration") + ) { + return isNodeOfType(parent.parent, "Program"); + } + return false; +}; + +// Walks `body` without descending into nested FunctionDeclaration / +// FunctionExpression / ArrowFunctionExpression / ClassBody children, so +// callbacks passed to `useEffect(() => { use... }, [])` don't accidentally +// register as a "real React hook call" — the only signal we care about is +// hooks called DIRECTLY inside the candidate hook's body. Returns the first +// CallExpression whose callee resolves to a React-hook-shaped name (`use` +// + uppercase / digit, or the bare React 19 `use`). Handles arrow +// expression bodies (`(cb) => useEffect(cb, [])`) by checking `body` +// itself as well as its descendants — without this check, a one-line +// reimplementation of `useMount` would slip past while the parenthesized +// variant `(cb) => (useEffect(cb, []))` would not. +const findReactHookCallInOwnBody = ( + body: EsTreeNode | null | undefined +): EsTreeNode | null => { + if (!body) return null; + let firstHookCall: EsTreeNode | null = null; + walkAst(body, (child) => { + if (firstHookCall) return false; + // Prune nested functions / classes so callbacks aren't mistaken for + // direct hook calls. The body itself bypasses this pruning so an + // arrow with a CallExpression body still gets checked. + if ( + child !== body && + (isNodeOfType(child, "FunctionDeclaration") || + isNodeOfType(child, "FunctionExpression") || + isNodeOfType(child, "ArrowFunctionExpression") || + isNodeOfType(child, "ClassDeclaration") || + isNodeOfType(child, "ClassExpression")) + ) { + return false; + } + if (isNodeOfType(child, "CallExpression")) { + const calleeName = getCalleeName(child); + if (calleeName && isReactHookName(calleeName)) { + firstHookCall = child; + return false; + } + } + return; + }); + return firstHookCall; +}; + +// A "delegation wrapper" hook is one whose body is a single statement that +// returns a call to the SAME canonical hook — either by name match +// (`function useDebounce(v) { return useDebounce(v, 500) }`) or via a +// renamed import from a known hook library +// (`import { useDebounce as upstream } from "react-use"; function +// useDebounce(v) { return upstream(v, 500) }`). The author is pre-binding +// the library hook, not reimplementing it; skipping these keeps the rule +// from flagging legitimate facade patterns. +const isDelegationWrapper = ( + body: EsTreeNode | null | undefined, + hookName: string +): boolean => { + if (!body) return false; + if (!isNodeOfType(body, "BlockStatement")) { + const expression = stripParenExpression(body); + return isCallToSameCanonicalHook(expression, hookName); + } + if (body.body.length !== 1) return false; + const onlyStatement = body.body[0]; + if (isNodeOfType(onlyStatement, "ReturnStatement")) { + if (!onlyStatement.argument) return false; + return isCallToSameCanonicalHook( + stripParenExpression(onlyStatement.argument), + hookName + ); + } + if (isNodeOfType(onlyStatement, "ExpressionStatement")) { + return isCallToSameCanonicalHook( + stripParenExpression(onlyStatement.expression), + hookName + ); + } + return false; +}; + +const isCallToSameCanonicalHook = ( + node: EsTreeNode, + hookName: string +): boolean => { + if (!isNodeOfType(node, "CallExpression")) return false; + if (!isNodeOfType(node.callee, "Identifier")) return false; + const calleeName = node.callee.name; + if (calleeName === hookName) return true; + for (const moduleSource of HOOK_LIBRARY_MODULE_SOURCES) { + if (getImportedNameFromModule(node, calleeName, moduleSource) === hookName) + return true; + } + return false; +}; + +const formatLibrarySuggestion = ( + availability: HookLibraryAvailability +): string => { + if (availability.reactUse && availability.usehooksTs) { + return "`react-use` or `usehooks-ts`"; + } + if (availability.reactUse) return "`react-use`"; + return "`usehooks-ts`"; +}; + +const buildDiagnosticMessage = ( + hookName: string, + availability: HookLibraryAvailability +): string => { + const librarySuggestion = formatLibrarySuggestion(availability); + return `\`${hookName}\` is a well-known hook already shipped by ${librarySuggestion}. Reimplementing it commonly misses SSR safety, cleanup races, stale closures from identity-unstable callbacks, or Strict-Mode double-fire — use the library version. If neither is installed, add \`react-use\` for the broader catalog.`; +}; + +const reportIfCandidateHookReimplementation = ( + context: RuleContext, + declarationNode: EsTreeNode, + identifierNode: EsTreeNode, + hookName: string, + body: EsTreeNode | null | undefined +): void => { + const availability = HOOK_LIBRARY_MAP.get(hookName); + if (!availability) return; + if (!isAtModuleScope(declarationNode)) return; + if (isDelegationWrapper(body, hookName)) return; + if (!findReactHookCallInOwnBody(body)) return; + context.report({ + node: identifierNode, + message: buildDiagnosticMessage(hookName, availability), + }); +}; + +// Catches top-level custom hooks whose names match a well-known hook from +// `react-use` or `usehooks-ts`. Detection is name-only by design — the +// hook map is curated to exclude ambiguous names (`useLocation`, +// `useEvent`, `useSearchParams`, `useNavigation`, `useEventCallback`, +// `useSpring`, etc.) so a same-name match almost always means the user +// hand-rolled the library hook. +// +// V1 scope: +// - Module-level FunctionDeclaration / VariableDeclarator only. +// - Body must call at least one React hook (filters out plain utilities +// that happen to start with `use`). +// - Skips single-statement delegation wrappers that just forward to a +// same-named hook (facade pattern, not a reimplementation). +// - Test files auto-skipped via the `test-noise` tag wrapper. +// +// Intentionally out of v1: +// - Pattern-based detection (e.g. `useEffect(() => setTimeout(...), [])` +// → suggest `useTimeout`). Different rule, much higher FP risk. +// - Hooks defined inside other components / hooks (factory patterns are +// usually intentional). +// - Re-export-only files (`export { useDebounce } from "./impl"` — +// there's no declarator to visit). +export const preferExistingHookLibrary = defineRule({ + id: "prefer-existing-hook-library", + title: "Custom hook reimplements a library hook", + tags: ["test-noise"], + severity: "warn", + category: "Architecture", + recommendation: + "Replace the hand-rolled hook with the same-named hook from `react-use` or `usehooks-ts`. Library hooks handle SSR safety, cleanup, Strict-Mode double-fire, and identity-unstable callbacks that hand-rolled versions usually miss.", + create: (context: RuleContext) => ({ + FunctionDeclaration(node: EsTreeNodeOfType<"FunctionDeclaration">) { + if (!node.id?.name) return; + reportIfCandidateHookReimplementation( + context, + node, + node.id, + node.id.name, + node.body + ); + }, + VariableDeclarator(node: EsTreeNodeOfType<"VariableDeclarator">) { + if (!isNodeOfType(node.id, "Identifier")) return; + const init = node.init; + if (!init) return; + if ( + !isNodeOfType(init, "ArrowFunctionExpression") && + !isNodeOfType(init, "FunctionExpression") + ) { + return; + } + // VariableDeclarator's grandparent is the module-scope check target: + // Program > VariableDeclaration > VariableDeclarator (optionally with + // an ExportNamedDeclaration wrapper above the VariableDeclaration). + const declarationParent = node.parent; + if (!declarationParent) return; + reportIfCandidateHookReimplementation( + context, + declarationParent, + node.id, + node.id.name, + init.body + ); + }, + }), +}); From c571bcdeb9be4337a825bf9a4f8b5a68b795115e Mon Sep 17 00:00:00 2001 From: Nisarg Patel Date: Fri, 5 Jun 2026 17:04:50 -0700 Subject: [PATCH 2/3] fix(rules): drop useDarkMode from prefer-existing-hook-library map The 50-entry RDE eval (tldraw/editor) confirmed what the name suggests: most codebases that ship a `useDarkMode` hook use it as a void DOM-side-effect that applies theme classes, not as a state toggle. Same name as `usehooks-ts/useDarkMode` but a wholly incompatible API (`useDarkMode(): void` vs `useDarkMode(): { isDarkMode, toggle, ... }`), so the match was a false positive every time it fired. Removed from HOOK_LIBRARY_MAP and pinned in place with a regression test + HACK comment in the exclusions block, alongside the `useLocation` / `useEvent` / `useEventCallback` exclusions. Users who actually want the toggle version typically import it directly from usehooks-ts anyway. Co-authored-by: Cursor --- .../src/plugin/constants/hook-libraries.ts | 9 ++++++- .../prefer-existing-hook-library.test.ts | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts index 8dd28acee..739ab28cd 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts @@ -37,6 +37,14 @@ export interface HookLibraryAvailability { // HACK: `useEventCallback` is omitted — both React-experimental and // usehooks-ts use the name with different semantics. We don't want to // recommend a swap on a hook that may be the React-native one. +// +// HACK: `useDarkMode` is omitted — the name is overloaded in the wild. The +// usehooks-ts version returns `{ isDarkMode, toggle, enable, disable, set }`, +// but most codebases that ship a `useDarkMode` hook use it as a void +// DOM-side-effect hook that applies theme classes (observed in tldraw, +// shadcn-style design systems, …). Same name, incompatible API → high FP +// rate. Users who do want the toggle version typically already import it +// directly from usehooks-ts. export const HOOK_LIBRARY_MAP: ReadonlyMap = new Map([ // Side-effects / timing @@ -165,7 +173,6 @@ export const HOOK_LIBRARY_MAP: ReadonlyMap = ["useTitle", { reactUse: true, usehooksTs: false }], ["useDocumentTitle", { reactUse: false, usehooksTs: true }], ["useFavicon", { reactUse: true, usehooksTs: false }], - ["useDarkMode", { reactUse: false, usehooksTs: true }], ["useTernaryDarkMode", { reactUse: false, usehooksTs: true }], // Clipboard / loading / scripts diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.test.ts index d4ae1da77..69cd17074 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-existing-hook-library.test.ts @@ -316,6 +316,30 @@ describe("prefer-existing-hook-library", () => { expect(result.diagnostics).toEqual([]); }); + // Regression: `useDarkMode` is intentionally NOT in HOOK_LIBRARY_MAP. + // The name is overloaded in the wild — most codebases ship a void + // DOM-effect hook (tldraw / shadcn-style design systems), which is + // semantically incompatible with usehooks-ts's `{ isDarkMode, toggle, … }` + // state hook. Keeping it out keeps the FP rate down. Don't add it back + // without first checking the v2 mitigations in `hook-libraries.ts`. + it("does not flag a `useDarkMode` hook (intentionally excluded from map)", () => { + const result = runRule( + preferExistingHookLibrary, + ` + import { useEffect } from "react"; + + export function useDarkMode() { + const colorMode = useColorMode(); + useEffect(() => { + document.documentElement.classList.toggle("dark", colorMode === "dark"); + }, [colorMode]); + } + ` + ); + + expect(result.diagnostics).toEqual([]); + }); + it("does not flag a PascalCase same-name binding", () => { const result = runRule( preferExistingHookLibrary, From 02223a2b2247d85a94951f765a181cda829644da Mon Sep 17 00:00:00 2001 From: Nisarg Patel Date: Fri, 5 Jun 2026 17:06:09 -0700 Subject: [PATCH 3/3] fix: manual --- .../src/plugin/constants/hook-libraries.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts index 739ab28cd..50b985a9c 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/hook-libraries.ts @@ -63,7 +63,7 @@ export const HOOK_LIBRARY_MAP: ReadonlyMap = ["useLocalStorage", { reactUse: true, usehooksTs: true }], ["useSessionStorage", { reactUse: true, usehooksTs: true }], ["useReadLocalStorage", { reactUse: false, usehooksTs: true }], - ["useCookie", { reactUse: true, usehooksTs: false }], + // ["useCookie", { reactUse: true, usehooksTs: false }], // not needed (human decided) // Lifecycle helpers ["useMount", { reactUse: true, usehooksTs: false }], @@ -83,7 +83,7 @@ export const HOOK_LIBRARY_MAP: ReadonlyMap = ["useFirstMountState", { reactUse: true, usehooksTs: false }], ["useIsFirstRender", { reactUse: false, usehooksTs: true }], ["useIsClient", { reactUse: false, usehooksTs: true }], - ["useSsr", { reactUse: false, usehooksTs: true }], + // ["useSsr", { reactUse: false, usehooksTs: true }], // not needed (human decided) // Previous / latest value ["usePrevious", { reactUse: true, usehooksTs: false }],