diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/react.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/react.ts index 4a0f0a870..fee9cf83f 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/react.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/react.ts @@ -55,6 +55,32 @@ export const REACT_HANDLER_PROP_PATTERN = /^on[A-Z]/; export const EFFECT_HOOK_NAMES = new Set(["useEffect", "useLayoutEffect"]); export const HOOKS_WITH_DEPS = new Set(["useEffect", "useLayoutEffect", "useMemo", "useCallback"]); +// Every stateful/effectful primitive hook React itself ships. A function +// that calls one of these is a genuine React hook implementation — used by +// `prefer-standard-hook` to distinguish a real hook reimplementation from an +// unrelated helper that merely happens to share a library hook's name. +export const REACT_BUILTIN_HOOK_NAMES = new Set([ + "useState", + "useReducer", + "useRef", + "useEffect", + "useLayoutEffect", + "useInsertionEffect", + "useMemo", + "useCallback", + "useContext", + "useImperativeHandle", + "useDebugValue", + "useId", + "useSyncExternalStore", + "useTransition", + "useDeferredValue", + "useOptimistic", + "useActionState", + "useFormStatus", + "useEffectEvent", +]); + // React's two component-wrapping HOCs that the rule visitor needs to // "see through" — `memo(Comp)` and `forwardRef(Comp)`. Both forms // (`memo` from a named import + `React.memo` via the namespace) are diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/standard-library-hooks.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/standard-library-hooks.ts new file mode 100644 index 000000000..d3810fa25 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/standard-library-hooks.ts @@ -0,0 +1,206 @@ +// The complete set of hook names shipped by `react-standard-hooks`, which +// re-exports the union of `react-use` (107 hooks) and `usehooks-ts` (33 hooks) +// — 127 distinct names. `prefer-standard-hook` matches a locally-defined hook +// against this set to recommend importing the battle-tested library version +// instead of hand-rolling it. Generated from the published packages: +// node -e "console.log([...new Set([...Object.keys(require('react-use')), +// ...Object.keys(require('usehooks-ts'))].filter(name => /^use[A-Z]/.test(name)))].sort())" +export const STANDARD_LIBRARY_HOOK_NAMES = new Set([ + "useAsync", + "useAsyncFn", + "useAsyncRetry", + "useAudio", + "useBattery", + "useBeforeUnload", + "useBoolean", + "useClickAnyWhere", + "useClickAway", + "useCookie", + "useCopyToClipboard", + "useCountdown", + "useCounter", + "useCss", + "useCustomCompareEffect", + "useDarkMode", + "useDebounce", + "useDebounceCallback", + "useDebounceValue", + "useDeepCompareEffect", + "useDefault", + "useDocumentTitle", + "useDrop", + "useDropArea", + "useEffectOnce", + "useEnsuredForwardedRef", + "useError", + "useEvent", + "useEventCallback", + "useEventListener", + "useFavicon", + "useFirstMountState", + "useFullscreen", + "useGeolocation", + "useGetSet", + "useGetSetState", + "useHarmonicIntervalFn", + "useHash", + "useHover", + "useHoverDirty", + "useIdle", + "useIntersection", + "useIntersectionObserver", + "useInterval", + "useIsClient", + "useIsMounted", + "useIsomorphicLayoutEffect", + "useKey", + "useKeyPress", + "useKeyPressEvent", + "useLatest", + "useLifecycles", + "useList", + "useLocalStorage", + "useLocation", + "useLockBodyScroll", + "useLogger", + "useLongPress", + "useMap", + "useMeasure", + "useMedia", + "useMediaDevices", + "useMediaQuery", + "useMediatedState", + "useMethods", + "useMotion", + "useMount", + "useMountedState", + "useMouse", + "useMouseHovered", + "useMouseWheel", + "useMultiStateValidator", + "useNetworkState", + "useNumber", + "useObservable", + "useOnClickOutside", + "useOrientation", + "usePageLeave", + "usePermission", + "usePinchZoom", + "usePrevious", + "usePreviousDistinct", + "usePromise", + "useQueue", + "useRaf", + "useRafLoop", + "useRafState", + "useReadLocalStorage", + "useRendersCount", + "useResizeObserver", + "useScratch", + "useScreen", + "useScript", + "useScroll", + "useScrollLock", + "useScrollbarWidth", + "useScrolling", + "useSearchParam", + "useSessionStorage", + "useSet", + "useSetState", + "useShallowCompareEffect", + "useSize", + "useSlider", + "useSpeech", + "useStartTyping", + "useStateList", + "useStateValidator", + "useStateWithHistory", + "useStep", + "useTernaryDarkMode", + "useThrottle", + "useThrottleFn", + "useTimeout", + "useTimeoutFn", + "useTitle", + "useToggle", + "useTween", + "useUnmount", + "useUnmountPromise", + "useUpdate", + "useUpdateEffect", + "useUpsert", + "useVibrate", + "useVideo", + "useWindowScroll", + "useWindowSize", +]); + +// Names that live in the library set above but are intentionally NOT +// reported. Each is a generic English word/concept that applications +// routinely use for an unrelated, app-specific hook, so a same-named local +// definition is far more often a coincidental name collision than a +// reimplementation of the library hook. Validated against ~50 OSS repos +// with react-doctor-evals, where the bare-word names below produced the +// rule's only false positives — e.g. `useScroll` reimplemented as a +// scrolled-past-threshold boolean (not react-use's `{x, y}` tracker), +// `useLocation` as a geo lat/lng editor (not the browser-location hook), +// `useSize`/`useScreen` as viewport-size hooks, `useVideo` as a context +// accessor, `useSpeech` as speech-to-text, `usePermission` as app +// authorization, `useCss` as a theme→CSS-string builder, and +// `useSearchParam`/`usePageLeave` built on the app's own router, and +// `useStep` as a wizard step-context accessor (not usehooks-ts's numeric +// step counter). Distinctive, +// purpose-specific names (useDebounce, useLocalStorage, useCopyToClipboard, +// useMediaQuery, usePrevious, useOnClickOutside, …) stay matched because a +// hand-rolled version of those is reliably the library hook. +// +// `useEvent` additionally collides with React's own experimental +// `useEvent` / `useEffectEvent` primitive, so a local one is usually that +// polyfill rather than react-use's DOM event-binding hook. +export const STANDARD_LIBRARY_HOOK_EXCLUSIONS = new Set([ + "useAudio", + "useCss", + "useDefault", + "useDrop", + "useDropArea", + "useError", + "useEvent", + "useGetSet", + "useGetSetState", + "useHash", + "useKey", + "useLatest", + "useList", + "useLocation", + "useLogger", + "useMap", + "useMeasure", + "useMedia", + "useMediatedState", + "useMethods", + "useMotion", + "useMouse", + "useMultiStateValidator", + "useNumber", + "useObservable", + "usePageLeave", + "usePermission", + "useQueue", + "useRaf", + "useScratch", + "useScreen", + "useScroll", + "useSearchParam", + "useSet", + "useSetState", + "useSize", + "useSlider", + "useSpeech", + "useStateList", + "useStateValidator", + "useStep", + "useTitle", + "useUpdate", + "useUpsert", + "useVideo", +]); 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 857a06c40..f5f385f47 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts @@ -232,6 +232,7 @@ import { preferHtmlDialog } from "./rules/a11y/prefer-html-dialog.js"; import { preferModuleScopePureFunction } from "./rules/architecture/prefer-module-scope-pure-function.js"; import { preferModuleScopeStaticValue } from "./rules/architecture/prefer-module-scope-static-value.js"; import { preferStableEmptyFallback } from "./rules/performance/prefer-stable-empty-fallback.js"; +import { preferStandardHook } from "./rules/architecture/prefer-standard-hook.js"; import { preferTagOverRole } from "./rules/a11y/prefer-tag-over-role.js"; import { preferUseEffectEvent } from "./rules/state-and-effects/prefer-use-effect-event.js"; import { preferUseSyncExternalStore } from "./rules/state-and-effects/prefer-use-sync-external-store.js"; @@ -2792,6 +2793,17 @@ export const reactDoctorRules = [ category: "Performance", }, }, + { + key: "react-doctor/prefer-standard-hook", + id: "prefer-standard-hook", + source: "react-doctor", + originallyExternal: false, + rule: { + ...preferStandardHook, + framework: "global", + category: "Architecture", + }, + }, { key: "react-doctor/prefer-tag-over-role", id: "prefer-tag-over-role", diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-standard-hook.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-standard-hook.test.ts new file mode 100644 index 000000000..4c5088e7d --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-standard-hook.test.ts @@ -0,0 +1,323 @@ +import { describe, expect, it } from "vite-plus/test"; +import { runRule } from "../../../test-utils/run-rule.js"; +import { preferStandardHook } from "./prefer-standard-hook.js"; + +describe("prefer-standard-hook", () => { + it("flags a function-declaration reimplementation of a library hook", () => { + const result = runRule( + preferStandardHook, + ` + import { useEffect, useState } from "react"; + + export function useDebounce(value, delay) { + const [debounced, setDebounced] = useState(value); + useEffect(() => { + const id = setTimeout(() => setDebounced(value), delay); + return () => clearTimeout(id); + }, [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-standard-hooks"); + expect(result.diagnostics[0].nodeType).toBe("Identifier"); + }); + + it("flags an arrow-function reimplementation assigned to a const", () => { + const result = runRule( + preferStandardHook, + ` + import { useState } from "react"; + + const useToggle = (initial = false) => { + const [value, setValue] = useState(initial); + const toggle = () => setValue((previous) => !previous); + return [value, toggle]; + }; + `, + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useToggle"); + }); + + it("flags a function-expression reimplementation assigned to a const", () => { + const result = runRule( + preferStandardHook, + ` + import { useEffect, useRef } from "react"; + + const usePrevious = function (value) { + const reference = useRef(); + useEffect(() => { + reference.current = value; + }); + return reference.current; + }; + `, + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("usePrevious"); + }); + + it("flags a reimplementation that calls React primitives via the namespace", () => { + const result = runRule( + preferStandardHook, + ` + import * as React from "react"; + + export function useLocalStorage(key, initial) { + const [value, setValue] = React.useState(initial); + React.useEffect(() => { + window.localStorage.setItem(key, JSON.stringify(value)); + }, [key, value]); + return [value, setValue]; + } + `, + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useLocalStorage"); + }); + + it("flags a reimplementation of a usehooks-ts-only hook", () => { + const result = runRule( + preferStandardHook, + ` + import { useEffect } from "react"; + + function useScrollLock() { + useEffect(() => { + const original = document.body.style.overflow; + document.body.style.overflow = "hidden"; + return () => { + document.body.style.overflow = original; + }; + }, []); + } + `, + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useScrollLock"); + }); + + it("flags a reimplementation built on useReducer", () => { + const result = runRule( + preferStandardHook, + ` + import { useReducer } from "react"; + + export const useCounter = (initial = 0) => { + const [count, dispatch] = useReducer((state, delta) => state + delta, initial); + return { count, increment: () => dispatch(1) }; + }; + `, + ); + + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("useCounter"); + }); + + it("does not flag importing and using the library hook", () => { + const result = runRule( + preferStandardHook, + ` + import { useDebounce } from "react-use"; + + export function SearchBox() { + useDebounce(() => {}, 200, []); + return null; + } + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag a thin wrapper that delegates to the library hook", () => { + const result = runRule( + preferStandardHook, + ` + import { useDebounce as useLibraryDebounce } from "react-use"; + + export const useDebounce = (callback, delay) => useLibraryDebounce(callback, delay, []); + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag a wrapper that adds local state but still delegates to the library", () => { + const result = runRule( + preferStandardHook, + ` + import { useState } from "react"; + import { useDebounce as useLibraryDebounce } from "react-use"; + + export function useDebounce(value, delay) { + const [ready, setReady] = useState(false); + useLibraryDebounce(() => setReady(true), delay, [value]); + return ready; + } + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag a same-named helper that is not actually a hook", () => { + const result = runRule( + preferStandardHook, + ` + const useMap = (entries) => new Map(entries); + export function useToggle(value) { + return !value; + } + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag hooks whose names are not in the library set", () => { + const result = runRule( + preferStandardHook, + ` + import { useState, useEffect } from "react"; + + export function useAuth() { + const [user, setUser] = useState(null); + useEffect(() => {}, []); + return user; + } + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag the excluded useEvent name (React useEffectEvent polyfill)", () => { + const result = runRule( + preferStandardHook, + ` + import { useCallback, useLayoutEffect, useRef } from "react"; + + export function useEvent(handler) { + const handlerRef = useRef(handler); + useLayoutEffect(() => { + handlerRef.current = handler; + }); + return useCallback((...args) => handlerRef.current(...args), []); + } + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag generic-word hook names that collide with app-specific hooks", () => { + const result = runRule( + preferStandardHook, + ` + import { useState, useCallback } from "react"; + + export function useScroll(threshold) { + const [scrolled, setScrolled] = useState(false); + const onScroll = useCallback(() => setScrolled(window.scrollY > threshold), [threshold]); + return scrolled; + } + + export const useLocation = (initial) => { + const [position, setPosition] = useState(initial); + return [position, setPosition]; + }; + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag a non-function binding (conditional layout-effect alias)", () => { + const result = runRule( + preferStandardHook, + ` + import { useEffect, useLayoutEffect } from "react"; + + export const useIsomorphicLayoutEffect = + typeof window !== "undefined" ? useLayoutEffect : useEffect; + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag an object method that shares a library hook name", () => { + const result = runRule( + preferStandardHook, + ` + import { useState } from "react"; + + const hooks = { + useToggle() { + const [value, setValue] = useState(false); + return [value, setValue]; + }, + }; + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag a hook that only calls another custom hook (v1 requires a React primitive)", () => { + const result = runRule( + preferStandardHook, + ` + export function usePrevious(value) { + return useTrackedValue(value); + } + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag a reimplementation using an aliased React primitive (documented v1 non-goal)", () => { + const result = runRule( + preferStandardHook, + ` + import { useState as useReactState } from "react"; + + export const useToggle = (initial = false) => { + const [value, setValue] = useReactState(initial); + return [value, () => setValue((previous) => !previous)]; + }; + `, + ); + + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag in test-like files", () => { + const result = runRule( + preferStandardHook, + ` + import { useState } from "react"; + + export function useToggle(initial) { + const [value, setValue] = useState(initial); + return [value, setValue]; + } + `, + { filename: "use-toggle.test.tsx" }, + ); + + expect(result.diagnostics).toHaveLength(0); + }); +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-standard-hook.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-standard-hook.ts new file mode 100644 index 000000000..9b45f1c98 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/architecture/prefer-standard-hook.ts @@ -0,0 +1,84 @@ +import { + STANDARD_LIBRARY_HOOK_EXCLUSIONS, + STANDARD_LIBRARY_HOOK_NAMES, +} from "../../constants/standard-library-hooks.js"; +import { REACT_BUILTIN_HOOK_NAMES } from "../../constants/react.js"; +import { defineRule } from "../../utils/define-rule.js"; +import { getCalleeName } from "../../utils/get-callee-name.js"; +import { isImportedFromModule } 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 { walkAst } from "../../utils/walk-ast.js"; +import type { EsTreeNode } from "../../utils/es-tree-node.js"; +import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +const STANDARD_HOOK_LIBRARY_SOURCES = ["react-standard-hooks", "react-use", "usehooks-ts"]; + +const isStandardLibraryHookName = (name: string): boolean => + STANDARD_LIBRARY_HOOK_NAMES.has(name) && !STANDARD_LIBRARY_HOOK_EXCLUSIONS.has(name); + +// A genuine reimplementation calls a React primitive hook (useState, useEffect, +// …) to build its own behavior; a thin wrapper instead delegates to the +// library hook it imported. We require the former and reject the latter so the +// rule only fires on hand-rolled hooks, never on functions that already use the +// library or on unrelated helpers that merely share a library hook's name. +const functionBodyReimplementsStandardHook = (functionNode: EsTreeNode): boolean => { + let callsReactBuiltinHook = false; + let delegatesToStandardLibraryHook = false; + walkAst(functionNode, (node) => { + if (!isNodeOfType(node, "CallExpression")) return; + const calleeName = getCalleeName(node); + if (!calleeName) return; + if (REACT_BUILTIN_HOOK_NAMES.has(calleeName)) { + callsReactBuiltinHook = true; + } + if ( + STANDARD_HOOK_LIBRARY_SOURCES.some((source) => isImportedFromModule(node, calleeName, source)) + ) { + delegatesToStandardLibraryHook = true; + } + }); + return callsReactBuiltinHook && !delegatesToStandardLibraryHook; +}; + +const buildMessage = (hookName: string): string => + `"${hookName}" reimplements a standard hook — import it from react-standard-hooks (or react-use / usehooks-ts) instead of hand-rolling it`; + +export const preferStandardHook = defineRule({ + id: "prefer-standard-hook", + tags: ["test-noise"], + severity: "warn", + recommendation: + "Import the hook from react-standard-hooks (or the react-use / usehooks-ts library you already use) instead of reimplementing it — the library version is battle-tested against the edge cases hand-rolled hooks usually miss.", + create: (context: RuleContext) => { + const reportIfReimplementedStandardHook = ( + hookName: string, + functionNode: EsTreeNode, + reportNode: EsTreeNode, + ): void => { + if (!isReactHookName(hookName)) return; + if (!isStandardLibraryHookName(hookName)) return; + if (!functionBodyReimplementsStandardHook(functionNode)) return; + context.report({ node: reportNode, message: buildMessage(hookName) }); + }; + + return { + FunctionDeclaration(node: EsTreeNodeOfType<"FunctionDeclaration">) { + const hookName = node.id?.name; + if (!hookName) return; + reportIfReimplementedStandardHook(hookName, node, node.id ?? node); + }, + VariableDeclarator(node: EsTreeNodeOfType<"VariableDeclarator">) { + if (!isNodeOfType(node.id, "Identifier")) return; + if ( + !isNodeOfType(node.init, "ArrowFunctionExpression") && + !isNodeOfType(node.init, "FunctionExpression") + ) + return; + reportIfReimplementedStandardHook(node.id.name, node.init, node.id); + }, + }; + }, +});