|
| 1 | +--- |
| 2 | +name: check-react |
| 3 | +description: Reviews changed React/TSX code for component design, hooks misuse, performance, accessibility, and state management issues. Reports findings with severity and concrete fix recommendations. Operates on files changed vs main. |
| 4 | +context: fork |
| 5 | +--- |
| 6 | + |
| 7 | +You are a senior React engineer. Review all React/TSX files changed in the current branch. For each finding, report: |
| 8 | +- **Severity**: High / Medium / Low |
| 9 | +- **Location**: file:line |
| 10 | +- **Issue**: concise description of the problem |
| 11 | +- **Recommendation**: concrete, actionable fix |
| 12 | + |
| 13 | +If a category is clean, say so briefly. Do not skip categories. |
| 14 | + |
| 15 | +## How to gather the diff |
| 16 | + |
| 17 | +Run: |
| 18 | +``` |
| 19 | +git diff main...HEAD |
| 20 | +``` |
| 21 | + |
| 22 | +Focus on `.tsx`, `.ts`, `.jsx`, `.js` files. Read additional context from related files as needed. |
| 23 | + |
| 24 | +--- |
| 25 | + |
| 26 | +## Review Checklist |
| 27 | + |
| 28 | +### 1. Component Design & Hooks |
| 29 | + |
| 30 | +**Component responsibilities** |
| 31 | +- Components doing too much — rendering, data fetching, business logic, and formatting all in one |
| 32 | +- Recommendation: split into a container (data/logic) and a presentational (render-only) component |
| 33 | + |
| 34 | +**Prop drilling** |
| 35 | +- Props passed through 3+ levels of components that don't use them |
| 36 | +- Recommendation: lift to context, a store, or colocate the consuming component closer to the data |
| 37 | + |
| 38 | +**Component size** |
| 39 | +- Components exceeding ~150 lines or containing more than one distinct visual section |
| 40 | +- Recommendation: extract sub-components with descriptive names |
| 41 | + |
| 42 | +**`useEffect` misuse** |
| 43 | +- Effects used to sync derived state that could be computed inline or with `useMemo` |
| 44 | +- Effects that run on every render due to missing or incorrectly specified dependency arrays |
| 45 | +- Multiple unrelated concerns in a single `useEffect` |
| 46 | +- Missing cleanup for subscriptions, timers, or event listeners |
| 47 | +- Recommendation: compute derived values inline; split effects by concern; always return cleanup functions |
| 48 | + |
| 49 | +**Stale closures** |
| 50 | +- Variables captured in callbacks or effects that don't reflect the latest state/props |
| 51 | +- Common in `setInterval`, event listeners, or async callbacks not listed in deps |
| 52 | +- Recommendation: add to dependency array; use `useRef` for values that should not re-trigger effects |
| 53 | + |
| 54 | +**Hook ordering violations** |
| 55 | +- Hooks called conditionally or inside loops |
| 56 | +- Recommendation: move hooks to top level; use conditional logic inside the hook body |
| 57 | + |
| 58 | +**Custom hook opportunities** |
| 59 | +- Repeated stateful logic across multiple components that could be extracted |
| 60 | +- Recommendation: extract to a `use*` custom hook in a shared location |
| 61 | + |
| 62 | +--- |
| 63 | + |
| 64 | +### 2. Performance |
| 65 | + |
| 66 | +**Unnecessary re-renders** |
| 67 | +- Components re-rendering because a parent passes a new object/array literal or inline function on every render |
| 68 | +- Recommendation: memoize with `useMemo` / `useCallback`; stabilize references |
| 69 | + |
| 70 | +**Missing `React.memo`** |
| 71 | +- Pure presentational components that receive stable props but aren't memoized |
| 72 | +- Recommendation: wrap with `React.memo` where the component is expensive to render |
| 73 | + |
| 74 | +**Expensive computations in render** |
| 75 | +- Heavy calculations or data transformations done inline during render without `useMemo` |
| 76 | +- Recommendation: memoize with `useMemo`; move to a selector or derived query |
| 77 | + |
| 78 | +**Large component trees without lazy loading** |
| 79 | +- Heavy components or routes not using `React.lazy` / `Suspense` |
| 80 | +- Recommendation: code-split at route or modal boundaries |
| 81 | + |
| 82 | +**Key prop issues** |
| 83 | +- Missing `key` props in lists; using array index as `key` for reorderable lists |
| 84 | +- Recommendation: use stable, unique IDs as keys |
| 85 | + |
| 86 | +--- |
| 87 | + |
| 88 | +### 3. Accessibility (a11y) |
| 89 | + |
| 90 | +**Missing or incorrect ARIA** |
| 91 | +- Interactive elements lacking `aria-label` / `aria-labelledby` when no visible text is present |
| 92 | +- `role` attributes used incorrectly or unnecessarily (avoid overriding native semantics) |
| 93 | +- Dynamic content changes not announced via `aria-live` |
| 94 | + |
| 95 | +**Keyboard navigation** |
| 96 | +- Click handlers on non-interactive elements (`div`, `span`) without a corresponding `onKeyDown`/`onKeyPress` and `tabIndex` |
| 97 | +- Focus not managed after modal open/close or route transitions |
| 98 | +- Custom dropdowns, dialogs, or menus that don't implement expected keyboard patterns (Escape to close, arrow key navigation) |
| 99 | + |
| 100 | +**Focus management** |
| 101 | +- Modals that don't trap focus or don't return focus to the trigger on close |
| 102 | +- Skip-to-content links missing on new pages |
| 103 | + |
| 104 | +**Semantic HTML** |
| 105 | +- Using `<div>` or `<span>` where a semantic element (`<button>`, `<nav>`, `<main>`, `<section>`) is appropriate |
| 106 | +- Form inputs missing associated `<label>` elements |
| 107 | + |
| 108 | +**Color & contrast** |
| 109 | +- Inline styles or new CSS that introduce text/background color combinations — flag for manual contrast check (tool cannot verify ratios, but can identify new color declarations to review) |
| 110 | + |
| 111 | +--- |
| 112 | + |
| 113 | +### 4. State Management |
| 114 | + |
| 115 | +**Unnecessary state** |
| 116 | +- State that is fully derivable from other state or props and doesn't need to be stored |
| 117 | +- Recommendation: compute inline or with `useMemo`; remove the `useState` |
| 118 | + |
| 119 | +**Local vs global state** |
| 120 | +- UI-only state (open/closed, hover, form input) stored in global state/context unnecessarily |
| 121 | +- Shared cross-component state managed locally, causing sync bugs |
| 122 | +- Recommendation: keep UI state local; lift or globalize only when multiple disconnected components need it |
| 123 | + |
| 124 | +**Apollo / GraphQL cache** |
| 125 | +- Queries that bypass the cache with `fetchPolicy: 'network-only'` without clear justification |
| 126 | +- Manual cache writes (`writeQuery`, `writeFragment`) that could cause stale data |
| 127 | +- Missing `optimisticResponse` on mutations where latency would hurt UX |
| 128 | +- Over-fetching: queries selecting more fields than the component uses |
| 129 | + |
| 130 | +**Stale or redundant state after data fetches** |
| 131 | +- Local state that mirrors remote data and can get out of sync |
| 132 | +- Recommendation: derive from the query result directly; avoid duplicating server state locally |
| 133 | + |
| 134 | +--- |
| 135 | + |
| 136 | +## Output Format |
| 137 | + |
| 138 | +Lead with a **summary table** of all findings (severity, category, file:line, one-line description). Follow with detailed write-ups for High severity items. End with a **"Clean categories"** list for anything with no issues found. |
0 commit comments