From e6749b29328f139047e28493b2517ba0fe9c3fca Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 08:16:58 +0000 Subject: [PATCH 1/3] feat(oxlint-plugin): add `client-prefer-keybind-library` rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flags manual addEventListener('keydown'|'keyup'|'keypress', …) calls on window/document or inside useEffect/useLayoutEffect callbacks, and recommends using a keybind library (react-hotkeys-hook, tinykeys, mousetrap, etc.) instead. The rule: - Detects global keyboard event listeners (window/document) - Detects keyboard listeners inside React effect hooks - Skips files that already import from a known keybind library - Skips non-keyboard events (click, scroll, resize, etc.) - Skips element-scoped listeners outside effects - Tagged test-noise to auto-skip test/story/spec files Includes 35 tests covering: - Positive cases (all keyboard event types, global/document/effect) - False positive avoidance (non-keyboard events, existing imports) - Open-source repo patterns (Cmd+K search, Escape-to-close, arrow nav) - Keybind library detection (react-hotkeys-hook, tinykeys, mousetrap, etc.) - Test file skipping Co-Authored-By: nisarg@million.dev --- .../src/plugin/constants/dom.ts | 14 + .../src/plugin/rule-registry.ts | 15 + .../client-prefer-keybind-library.test.ts | 543 ++++++++++++++++++ .../client/client-prefer-keybind-library.ts | 96 ++++ 4 files changed, 668 insertions(+) create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.test.ts create mode 100644 packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/dom.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/dom.ts index d1257026d..40a6f8933 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/dom.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/dom.ts @@ -65,3 +65,17 @@ export const EXTERNAL_SYNC_OBSERVER_CONSTRUCTORS = new Set([ ]); export const STORAGE_OBJECTS = new Set(["localStorage", "sessionStorage"]); + +export const KEYBOARD_EVENT_NAMES = new Set(["keydown", "keyup", "keypress"]); + +export const KEYBIND_LIBRARY_PACKAGES: ReadonlyArray = [ + "react-hotkeys-hook", + "react-hotkeys", + "@mantine/hooks", + "tinykeys", + "hotkeys-js", + "mousetrap", + "@react-hook/hotkey", + "use-hotkeys", + "@invoke-ai/ui-library", +]; 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 3c304487e..5a12cd66e 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts @@ -27,6 +27,7 @@ import { checkedRequiresOnchangeOrReadonly } from "./rules/react-builtins/checke import { clickEventsHaveKeyEvents } from "./rules/a11y/click-events-have-key-events.js"; import { clientLocalstorageNoVersion } from "./rules/client/client-localstorage-no-version.js"; import { clientPassiveEventListeners } from "./rules/client/client-passive-event-listeners.js"; +import { clientPreferKeybindLibrary } from "./rules/client/client-prefer-keybind-library.js"; import { controlHasAssociatedLabel } from "./rules/a11y/control-has-associated-label.js"; import { noBoldHeading } from "./rules/react-ui/no-bold-heading.js"; import { noEmDashInJsxText } from "./rules/react-ui/no-em-dash-in-jsx-text.js"; @@ -562,6 +563,20 @@ export const reactDoctorRules = [ category: "Performance", }, }, + { + key: "react-doctor/client-prefer-keybind-library", + id: "client-prefer-keybind-library", + source: "react-doctor", + originallyExternal: false, + framework: "global", + category: "Performance", + severity: "warn", + rule: { + ...clientPreferKeybindLibrary, + framework: "global", + category: "Performance", + }, + }, { key: "react-doctor/control-has-associated-label", id: "control-has-associated-label", diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.test.ts new file mode 100644 index 000000000..5c7ce6dd8 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.test.ts @@ -0,0 +1,543 @@ +import { describe, expect, it } from "vite-plus/test"; +import { runRule } from "../../../test-utils/run-rule.js"; +import { clientPreferKeybindLibrary } from "./client-prefer-keybind-library.js"; + +describe("client-prefer-keybind-library", () => { + describe("flags manual keyboard event listeners", () => { + it("flags window.addEventListener('keydown', handler)", () => { + const code = ` + const App = () => { + useEffect(() => { + const handler = (e) => { + if (e.key === 'k' && e.metaKey) openSearch(); + }; + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("react-hotkeys-hook"); + expect(result.diagnostics[0].message).toContain("keydown"); + }); + + it("flags document.addEventListener('keydown', handler)", () => { + const code = ` + const App = () => { + useEffect(() => { + document.addEventListener('keydown', handleKeyDown); + return () => document.removeEventListener('keydown', handleKeyDown); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("document"); + }); + + it("flags window.addEventListener('keyup', handler)", () => { + const code = ` + const App = () => { + useEffect(() => { + window.addEventListener('keyup', handler); + return () => window.removeEventListener('keyup', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("keyup"); + }); + + it("flags window.addEventListener('keypress', handler)", () => { + const code = ` + const App = () => { + useEffect(() => { + window.addEventListener('keypress', handler); + return () => window.removeEventListener('keypress', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + expect(result.diagnostics[0].message).toContain("keypress"); + }); + + it("flags global keydown listener outside useEffect", () => { + const code = ` + window.addEventListener('keydown', (e) => { + if (e.key === 'Escape') closeModal(); + }); + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + + it("flags document keydown listener outside useEffect", () => { + const code = ` + document.addEventListener('keydown', (e) => { + if (e.key === 'Escape') closeModal(); + }); + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + + it("flags keydown in useLayoutEffect", () => { + const code = ` + const App = () => { + useLayoutEffect(() => { + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + + it("flags multiple keyboard listeners in the same effect", () => { + const code = ` + const App = () => { + useEffect(() => { + window.addEventListener('keydown', onKeyDown); + window.addEventListener('keyup', onKeyUp); + return () => { + window.removeEventListener('keydown', onKeyDown); + window.removeEventListener('keyup', onKeyUp); + }; + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(2); + }); + }); + + describe("does not flag non-keyboard event listeners", () => { + it("ignores window.addEventListener('click', handler)", () => { + const code = ` + const App = () => { + useEffect(() => { + window.addEventListener('click', handler); + return () => window.removeEventListener('click', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("ignores window.addEventListener('scroll', handler)", () => { + const code = ` + const App = () => { + useEffect(() => { + window.addEventListener('scroll', handler); + return () => window.removeEventListener('scroll', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("ignores window.addEventListener('resize', handler)", () => { + const code = ` + const App = () => { + useEffect(() => { + window.addEventListener('resize', handler); + return () => window.removeEventListener('resize', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("ignores addEventListener('mousedown', handler)", () => { + const code = ` + const App = () => { + useEffect(() => { + document.addEventListener('mousedown', handler); + return () => document.removeEventListener('mousedown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + }); + + describe("does not flag when a keybind library is already imported", () => { + it("skips when react-hotkeys-hook is imported", () => { + const code = ` + import { useHotkeys } from 'react-hotkeys-hook'; + const App = () => { + useEffect(() => { + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("skips when tinykeys is imported", () => { + const code = ` + import tinykeys from 'tinykeys'; + const App = () => { + useEffect(() => { + document.addEventListener('keydown', handler); + return () => document.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("skips when hotkeys-js is imported", () => { + const code = ` + import hotkeys from 'hotkeys-js'; + const App = () => { + useEffect(() => { + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("skips when mousetrap is imported", () => { + const code = ` + import Mousetrap from 'mousetrap'; + const App = () => { + useEffect(() => { + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("skips when react-hotkeys is imported", () => { + const code = ` + import { HotKeys } from 'react-hotkeys'; + const App = () => { + useEffect(() => { + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("skips when @mantine/hooks is imported", () => { + const code = ` + import { useHotkeys } from '@mantine/hooks'; + const App = () => { + useEffect(() => { + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("skips subpath imports from a keybind library", () => { + const code = ` + import { useHotkeys } from 'react-hotkeys-hook/dist/index'; + const App = () => { + useEffect(() => { + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + }); + + describe("does not flag element-scoped listeners (non-global, non-effect)", () => { + it("ignores ref.current.addEventListener('keydown', handler) outside useEffect", () => { + const code = ` + const handler = (e) => { console.log(e.key); }; + inputRef.current.addEventListener('keydown', handler); + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("ignores element.addEventListener('keydown', handler) in non-effect function", () => { + const code = ` + const setupKeybinds = (element) => { + element.addEventListener('keydown', handler); + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + }); + + describe("flags element-scoped listeners inside useEffect", () => { + it("flags ref.current.addEventListener('keydown', handler) inside useEffect", () => { + const code = ` + const App = () => { + const inputRef = useRef(null); + useEffect(() => { + inputRef.current.addEventListener('keydown', handler); + return () => inputRef.current.removeEventListener('keydown', handler); + }, []); + return ; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + }); + + describe("does not flag unrelated patterns", () => { + it("ignores addEventListener with dynamic event name", () => { + const code = ` + const App = () => { + useEffect(() => { + window.addEventListener(eventName, handler); + return () => window.removeEventListener(eventName, handler); + }, [eventName]); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("ignores addEventListener with template literal event name", () => { + const code = ` + const App = () => { + useEffect(() => { + window.addEventListener(\`key\${type}\`, handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("ignores non-addEventListener member calls with 'keydown'", () => { + const code = ` + const App = () => { + useEffect(() => { + emitter.on('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + + it("ignores addEventListener with fewer than 2 arguments", () => { + const code = ` + window.addEventListener('keydown'); + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(0); + }); + }); + + describe("open-source repo patterns", () => { + it("flags VS Code-style Ctrl+Shift+P command palette shortcut", () => { + const code = ` + const CommandPalette = () => { + const [isOpen, setIsOpen] = useState(false); + useEffect(() => { + const handleKeyDown = (event) => { + if (event.ctrlKey && event.shiftKey && event.key === 'p') { + event.preventDefault(); + setIsOpen(true); + } + }; + window.addEventListener('keydown', handleKeyDown); + return () => window.removeEventListener('keydown', handleKeyDown); + }, []); + return isOpen ? : null; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + + it("flags Cmd+K search shortcut pattern", () => { + const code = ` + const SearchBar = () => { + useEffect(() => { + const onKeyDown = (e) => { + if ((e.metaKey || e.ctrlKey) && e.key === 'k') { + e.preventDefault(); + openSearch(); + } + }; + document.addEventListener('keydown', onKeyDown); + return () => document.removeEventListener('keydown', onKeyDown); + }, []); + return ; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + + it("flags Escape-to-close modal pattern", () => { + const code = ` + const Modal = ({ onClose }) => { + useEffect(() => { + const handleEscape = (e) => { + if (e.key === 'Escape') onClose(); + }; + document.addEventListener('keydown', handleEscape); + return () => document.removeEventListener('keydown', handleEscape); + }, [onClose]); + return
Content
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + + it("flags arrow-key navigation pattern", () => { + const code = ` + const List = ({ items }) => { + const [selectedIndex, setSelectedIndex] = useState(0); + useEffect(() => { + const handleArrowKeys = (e) => { + if (e.key === 'ArrowDown') setSelectedIndex(i => Math.min(i + 1, items.length - 1)); + if (e.key === 'ArrowUp') setSelectedIndex(i => Math.max(i - 1, 0)); + }; + window.addEventListener('keydown', handleArrowKeys); + return () => window.removeEventListener('keydown', handleArrowKeys); + }, [items.length]); + return
    {items.map((item, i) =>
  • {item}
  • )}
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + + it("flags multi-key shortcut table pattern", () => { + const code = ` + const App = () => { + useEffect(() => { + const shortcuts = { + 'ctrl+s': save, + 'ctrl+z': undo, + 'ctrl+y': redo, + }; + const handler = (e) => { + const key = [e.ctrlKey && 'ctrl', e.key].filter(Boolean).join('+'); + if (shortcuts[key]) { + e.preventDefault(); + shortcuts[key](); + } + }; + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return ; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + + it("flags focus-trap keyboard handler pattern", () => { + const code = ` + const Dialog = ({ children }) => { + const dialogRef = useRef(null); + useEffect(() => { + const trapFocus = (e) => { + if (e.key === 'Tab') { + const focusable = dialogRef.current.querySelectorAll('button, input'); + if (e.shiftKey && document.activeElement === focusable[0]) { + e.preventDefault(); + focusable[focusable.length - 1].focus(); + } + } + }; + document.addEventListener('keydown', trapFocus); + return () => document.removeEventListener('keydown', trapFocus); + }, []); + return
{children}
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code); + expect(result.diagnostics).toHaveLength(1); + }); + }); + + describe("test file skipping (test-noise tag)", () => { + it("does not flag in test files", () => { + const code = ` + const App = () => { + useEffect(() => { + window.addEventListener('keydown', handler); + return () => window.removeEventListener('keydown', handler); + }, []); + return
; + }; + `; + const result = runRule(clientPreferKeybindLibrary, code, { + filename: "App.test.tsx", + }); + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag in spec files", () => { + const code = ` + window.addEventListener('keydown', handler); + `; + const result = runRule(clientPreferKeybindLibrary, code, { + filename: "keyboard.spec.ts", + }); + expect(result.diagnostics).toHaveLength(0); + }); + + it("does not flag in story files", () => { + const code = ` + window.addEventListener('keydown', handler); + `; + const result = runRule(clientPreferKeybindLibrary, code, { + filename: "Modal.stories.tsx", + }); + expect(result.diagnostics).toHaveLength(0); + }); + }); +}); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts new file mode 100644 index 000000000..9e20a6488 --- /dev/null +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts @@ -0,0 +1,96 @@ +import { KEYBOARD_EVENT_NAMES, KEYBIND_LIBRARY_PACKAGES } from "../../constants/dom.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 { isNodeOfType } from "../../utils/is-node-of-type.js"; +import { isMemberProperty } from "../../utils/is-member-property.js"; +import type { Rule } from "../../utils/rule.js"; +import type { RuleContext } from "../../utils/rule-context.js"; + +const GLOBAL_LISTENER_OBJECTS = new Set(["window", "document"]); + +const detectExistingKeybindLibrary = (programNode: EsTreeNodeOfType<"Program">): string | null => { + for (const statement of programNode.body ?? []) { + if (!isNodeOfType(statement, "ImportDeclaration")) continue; + const source = statement.source?.value; + if (typeof source !== "string") continue; + for (const knownPackage of KEYBIND_LIBRARY_PACKAGES) { + if (source === knownPackage || source.startsWith(`${knownPackage}/`)) { + return knownPackage; + } + } + } + return null; +}; + +const isInsideUseEffectCallback = (node: EsTreeNode): boolean => { + let current: EsTreeNode | null | undefined = node.parent; + while (current) { + if ( + isNodeOfType(current, "CallExpression") && + isNodeOfType(current.callee, "Identifier") && + (current.callee.name === "useEffect" || current.callee.name === "useLayoutEffect") + ) { + return true; + } + current = current.parent; + } + return false; +}; + +const buildRecommendationMessage = ( + eventName: string, + receiverName: string | null, +): string => { + const receiverPrefix = receiverName ? `${receiverName}.` : ""; + return `${receiverPrefix}addEventListener("${eventName}", …) registers a manual keyboard shortcut — use a keybind library like react-hotkeys-hook instead for consistent, declarative, and accessible keyboard shortcut management`; +}; + +export const clientPreferKeybindLibrary = defineRule({ + id: "client-prefer-keybind-library", + tags: ["test-noise"], + severity: "warn", + recommendation: + 'Use a keybind library like react-hotkeys-hook (`useHotkeys("mod+k", handler)`) instead of manual addEventListener("keydown", …) — it handles focus scoping, modifier normalization, and cleanup automatically', + create: (context: RuleContext) => { + let fileHasKeybindLibrary = false; + + return { + Program(node: EsTreeNodeOfType<"Program">) { + fileHasKeybindLibrary = detectExistingKeybindLibrary(node) !== null; + }, + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (fileHasKeybindLibrary) return; + if (!isMemberProperty(node.callee, "addEventListener")) return; + if ((node.arguments?.length ?? 0) < 2) return; + + const eventNameNode = node.arguments[0]; + if ( + !isNodeOfType(eventNameNode, "Literal") || + typeof eventNameNode.value !== "string" || + !KEYBOARD_EVENT_NAMES.has(eventNameNode.value) + ) { + return; + } + + const callee = node.callee; + if (!isNodeOfType(callee, "MemberExpression")) return; + + const isGlobalReceiver = + isNodeOfType(callee.object, "Identifier") && + GLOBAL_LISTENER_OBJECTS.has(callee.object.name); + const isEffectBound = isInsideUseEffectCallback(node); + + if (!isGlobalReceiver && !isEffectBound) return; + + const receiverName = + isNodeOfType(callee.object, "Identifier") ? callee.object.name : null; + + context.report({ + node, + message: buildRecommendationMessage(eventNameNode.value, receiverName), + }); + }, + }; + }, +}); From 5a9c8ae57ae6cd4398be862f86ac7c5e6afffd8e Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 25 May 2026 08:22:49 +0000 Subject: [PATCH 2/3] style: apply formatter to rule file Co-Authored-By: nisarg@million.dev --- .../plugin/rules/client/client-prefer-keybind-library.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts index 9e20a6488..1956d00d4 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts @@ -38,10 +38,7 @@ const isInsideUseEffectCallback = (node: EsTreeNode): boolean => { return false; }; -const buildRecommendationMessage = ( - eventName: string, - receiverName: string | null, -): string => { +const buildRecommendationMessage = (eventName: string, receiverName: string | null): string => { const receiverPrefix = receiverName ? `${receiverName}.` : ""; return `${receiverPrefix}addEventListener("${eventName}", …) registers a manual keyboard shortcut — use a keybind library like react-hotkeys-hook instead for consistent, declarative, and accessible keyboard shortcut management`; }; @@ -83,8 +80,7 @@ export const clientPreferKeybindLibrary = defineRule({ if (!isGlobalReceiver && !isEffectBound) return; - const receiverName = - isNodeOfType(callee.object, "Identifier") ? callee.object.name : null; + const receiverName = isNodeOfType(callee.object, "Identifier") ? callee.object.name : null; context.report({ node, From ed5221b92a987186a8aa9bd1c82fe9bae64f67a5 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 27 May 2026 06:14:22 +0000 Subject: [PATCH 3/3] =?UTF-8?q?refactor:=20address=20review=20=E2=80=94=20?= =?UTF-8?q?category=20to=20Architecture,=20remove=20library=20skip?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Change category from Performance to Architecture per reviewer feedback - Remove KEYBIND_LIBRARY_PACKAGES constant and library-detection logic — flag all manual keyboard listeners unconditionally to prefer false positives over false negatives - Update tests accordingly (30 tests, all pass) Co-Authored-By: nisarg@million.dev --- .../src/plugin/constants/dom.ts | 12 --- .../src/plugin/rule-registry.ts | 4 +- .../client-prefer-keybind-library.test.ts | 85 ++----------------- .../client/client-prefer-keybind-library.ts | 77 ++++++----------- 4 files changed, 35 insertions(+), 143 deletions(-) diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/constants/dom.ts b/packages/oxlint-plugin-react-doctor/src/plugin/constants/dom.ts index 40a6f8933..a6f09deef 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/constants/dom.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/constants/dom.ts @@ -67,15 +67,3 @@ export const EXTERNAL_SYNC_OBSERVER_CONSTRUCTORS = new Set([ export const STORAGE_OBJECTS = new Set(["localStorage", "sessionStorage"]); export const KEYBOARD_EVENT_NAMES = new Set(["keydown", "keyup", "keypress"]); - -export const KEYBIND_LIBRARY_PACKAGES: ReadonlyArray = [ - "react-hotkeys-hook", - "react-hotkeys", - "@mantine/hooks", - "tinykeys", - "hotkeys-js", - "mousetrap", - "@react-hook/hotkey", - "use-hotkeys", - "@invoke-ai/ui-library", -]; 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 5a12cd66e..1af30aabb 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rule-registry.ts @@ -569,12 +569,12 @@ export const reactDoctorRules = [ source: "react-doctor", originallyExternal: false, framework: "global", - category: "Performance", + category: "Architecture", severity: "warn", rule: { ...clientPreferKeybindLibrary, framework: "global", - category: "Performance", + category: "Architecture", }, }, { diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.test.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.test.ts index 5c7ce6dd8..dbad8b30d 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.test.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.test.ts @@ -179,8 +179,8 @@ describe("client-prefer-keybind-library", () => { }); }); - describe("does not flag when a keybind library is already imported", () => { - it("skips when react-hotkeys-hook is imported", () => { + describe("still flags even when a keybind library is imported", () => { + it("flags even when react-hotkeys-hook is imported", () => { const code = ` import { useHotkeys } from 'react-hotkeys-hook'; const App = () => { @@ -192,10 +192,10 @@ describe("client-prefer-keybind-library", () => { }; `; const result = runRule(clientPreferKeybindLibrary, code); - expect(result.diagnostics).toHaveLength(0); + expect(result.diagnostics).toHaveLength(1); }); - it("skips when tinykeys is imported", () => { + it("flags even when tinykeys is imported", () => { const code = ` import tinykeys from 'tinykeys'; const App = () => { @@ -207,82 +207,7 @@ describe("client-prefer-keybind-library", () => { }; `; const result = runRule(clientPreferKeybindLibrary, code); - expect(result.diagnostics).toHaveLength(0); - }); - - it("skips when hotkeys-js is imported", () => { - const code = ` - import hotkeys from 'hotkeys-js'; - const App = () => { - useEffect(() => { - window.addEventListener('keydown', handler); - return () => window.removeEventListener('keydown', handler); - }, []); - return
; - }; - `; - const result = runRule(clientPreferKeybindLibrary, code); - expect(result.diagnostics).toHaveLength(0); - }); - - it("skips when mousetrap is imported", () => { - const code = ` - import Mousetrap from 'mousetrap'; - const App = () => { - useEffect(() => { - window.addEventListener('keydown', handler); - return () => window.removeEventListener('keydown', handler); - }, []); - return
; - }; - `; - const result = runRule(clientPreferKeybindLibrary, code); - expect(result.diagnostics).toHaveLength(0); - }); - - it("skips when react-hotkeys is imported", () => { - const code = ` - import { HotKeys } from 'react-hotkeys'; - const App = () => { - useEffect(() => { - window.addEventListener('keydown', handler); - return () => window.removeEventListener('keydown', handler); - }, []); - return
; - }; - `; - const result = runRule(clientPreferKeybindLibrary, code); - expect(result.diagnostics).toHaveLength(0); - }); - - it("skips when @mantine/hooks is imported", () => { - const code = ` - import { useHotkeys } from '@mantine/hooks'; - const App = () => { - useEffect(() => { - window.addEventListener('keydown', handler); - return () => window.removeEventListener('keydown', handler); - }, []); - return
; - }; - `; - const result = runRule(clientPreferKeybindLibrary, code); - expect(result.diagnostics).toHaveLength(0); - }); - - it("skips subpath imports from a keybind library", () => { - const code = ` - import { useHotkeys } from 'react-hotkeys-hook/dist/index'; - const App = () => { - useEffect(() => { - window.addEventListener('keydown', handler); - return () => window.removeEventListener('keydown', handler); - }, []); - return
; - }; - `; - const result = runRule(clientPreferKeybindLibrary, code); - expect(result.diagnostics).toHaveLength(0); + expect(result.diagnostics).toHaveLength(1); }); }); diff --git a/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts index 1956d00d4..2e93b3b58 100644 --- a/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts +++ b/packages/oxlint-plugin-react-doctor/src/plugin/rules/client/client-prefer-keybind-library.ts @@ -1,4 +1,4 @@ -import { KEYBOARD_EVENT_NAMES, KEYBIND_LIBRARY_PACKAGES } from "../../constants/dom.js"; +import { KEYBOARD_EVENT_NAMES } from "../../constants/dom.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"; @@ -9,20 +9,6 @@ import type { RuleContext } from "../../utils/rule-context.js"; const GLOBAL_LISTENER_OBJECTS = new Set(["window", "document"]); -const detectExistingKeybindLibrary = (programNode: EsTreeNodeOfType<"Program">): string | null => { - for (const statement of programNode.body ?? []) { - if (!isNodeOfType(statement, "ImportDeclaration")) continue; - const source = statement.source?.value; - if (typeof source !== "string") continue; - for (const knownPackage of KEYBIND_LIBRARY_PACKAGES) { - if (source === knownPackage || source.startsWith(`${knownPackage}/`)) { - return knownPackage; - } - } - } - return null; -}; - const isInsideUseEffectCallback = (node: EsTreeNode): boolean => { let current: EsTreeNode | null | undefined = node.parent; while (current) { @@ -47,46 +33,39 @@ export const clientPreferKeybindLibrary = defineRule({ id: "client-prefer-keybind-library", tags: ["test-noise"], severity: "warn", + category: "Architecture", recommendation: 'Use a keybind library like react-hotkeys-hook (`useHotkeys("mod+k", handler)`) instead of manual addEventListener("keydown", …) — it handles focus scoping, modifier normalization, and cleanup automatically', - create: (context: RuleContext) => { - let fileHasKeybindLibrary = false; - - return { - Program(node: EsTreeNodeOfType<"Program">) { - fileHasKeybindLibrary = detectExistingKeybindLibrary(node) !== null; - }, - CallExpression(node: EsTreeNodeOfType<"CallExpression">) { - if (fileHasKeybindLibrary) return; - if (!isMemberProperty(node.callee, "addEventListener")) return; - if ((node.arguments?.length ?? 0) < 2) return; + create: (context: RuleContext) => ({ + CallExpression(node: EsTreeNodeOfType<"CallExpression">) { + if (!isMemberProperty(node.callee, "addEventListener")) return; + if ((node.arguments?.length ?? 0) < 2) return; - const eventNameNode = node.arguments[0]; - if ( - !isNodeOfType(eventNameNode, "Literal") || - typeof eventNameNode.value !== "string" || - !KEYBOARD_EVENT_NAMES.has(eventNameNode.value) - ) { - return; - } + const eventNameNode = node.arguments[0]; + if ( + !isNodeOfType(eventNameNode, "Literal") || + typeof eventNameNode.value !== "string" || + !KEYBOARD_EVENT_NAMES.has(eventNameNode.value) + ) { + return; + } - const callee = node.callee; - if (!isNodeOfType(callee, "MemberExpression")) return; + const callee = node.callee; + if (!isNodeOfType(callee, "MemberExpression")) return; - const isGlobalReceiver = - isNodeOfType(callee.object, "Identifier") && - GLOBAL_LISTENER_OBJECTS.has(callee.object.name); - const isEffectBound = isInsideUseEffectCallback(node); + const isGlobalReceiver = + isNodeOfType(callee.object, "Identifier") && + GLOBAL_LISTENER_OBJECTS.has(callee.object.name); + const isEffectBound = isInsideUseEffectCallback(node); - if (!isGlobalReceiver && !isEffectBound) return; + if (!isGlobalReceiver && !isEffectBound) return; - const receiverName = isNodeOfType(callee.object, "Identifier") ? callee.object.name : null; + const receiverName = isNodeOfType(callee.object, "Identifier") ? callee.object.name : null; - context.report({ - node, - message: buildRecommendationMessage(eventNameNode.value, receiverName), - }); - }, - }; - }, + context.report({ + node, + message: buildRecommendationMessage(eventNameNode.value, receiverName), + }); + }, + }), });