| name | fxa-check-react |
| 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. |
| context | fork |
You are a senior React engineer. Review all React/TSX files changed in the current branch. For each finding, report:
- Severity: High / Medium / Low
- Location: file:line
- Issue: concise description of the problem
- Recommendation: concrete, actionable fix
If a category is clean, say so briefly. Do not skip categories.
How to gather the diff
Run:
git diff main...HEAD
Focus on .tsx, .ts, .jsx, .js files. Read additional context from related files as needed.
Review Checklist
1. Component Design & Hooks
Component responsibilities
- Components doing too much — rendering, data fetching, business logic, and formatting all in one
- Recommendation: split into a container (data/logic) and a presentational (render-only) component
Prop drilling
- Props passed through 3+ levels of components that don't use them
- Recommendation: lift to context, a store, or colocate the consuming component closer to the data
Component size
- Components exceeding ~150 lines or containing more than one distinct visual section
- Recommendation: extract sub-components with descriptive names
useEffect misuse
- Effects used to sync derived state that could be computed inline or with
useMemo
- Effects that run on every render due to missing or incorrectly specified dependency arrays
- Multiple unrelated concerns in a single
useEffect
- Missing cleanup for subscriptions, timers, or event listeners
- Recommendation: compute derived values inline; split effects by concern; always return cleanup functions
Stale closures
- Variables captured in callbacks or effects that don't reflect the latest state/props
- Common in
setInterval, event listeners, or async callbacks not listed in deps
- Recommendation: add to dependency array; use
useRef for values that should not re-trigger effects
Hook ordering violations
- Hooks called conditionally or inside loops
- Recommendation: move hooks to top level; use conditional logic inside the hook body
Custom hook opportunities
- Repeated stateful logic across multiple components that could be extracted
- Recommendation: extract to a
use* custom hook in a shared location
2. Performance
Unnecessary re-renders
- Components re-rendering because a parent passes a new object/array literal or inline function on every render
- Recommendation: memoize with
useMemo / useCallback; stabilize references
Missing React.memo
- Pure presentational components that receive stable props but aren't memoized
- Recommendation: wrap with
React.memo where the component is expensive to render
Expensive computations in render
- Heavy calculations or data transformations done inline during render without
useMemo
- Recommendation: memoize with
useMemo; move to a selector or derived query
Large component trees without lazy loading
- Heavy components or routes not using
React.lazy / Suspense
- Recommendation: code-split at route or modal boundaries
Key prop issues
- Missing
key props in lists; using array index as key for reorderable lists
- Recommendation: use stable, unique IDs as keys
3. Accessibility (a11y)
Missing or incorrect ARIA
- Interactive elements lacking
aria-label / aria-labelledby when no visible text is present
role attributes used incorrectly or unnecessarily (avoid overriding native semantics)- Dynamic content changes not announced via
aria-live
Keyboard navigation
- Click handlers on non-interactive elements (
div, span) without a corresponding onKeyDown/onKeyPress and tabIndex
- Focus not managed after modal open/close or route transitions
- Custom dropdowns, dialogs, or menus that don't implement expected keyboard patterns (Escape to close, arrow key navigation)
Focus management
- Modals that don't trap focus or don't return focus to the trigger on close
- Skip-to-content links missing on new pages
Semantic HTML
- Using
<div> or <span> where a semantic element (<button>, <nav>, <main>, <section>) is appropriate
- Form inputs missing associated
<label> elements
Color & contrast
- 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)
4. State Management
Unnecessary state
- State that is fully derivable from other state or props and doesn't need to be stored
- Recommendation: compute inline or with
useMemo; remove the useState
Local vs global state
- UI-only state (open/closed, hover, form input) stored in global state/context unnecessarily
- Shared cross-component state managed locally, causing sync bugs
- Recommendation: keep UI state local; lift or globalize only when multiple disconnected components need it
Apollo / GraphQL cache
- Queries that bypass the cache with
fetchPolicy: 'network-only' without clear justification
- Manual cache writes (
writeQuery, writeFragment) that could cause stale data
- Missing
optimisticResponse on mutations where latency would hurt UX
- Over-fetching: queries selecting more fields than the component uses
Stale or redundant state after data fetches
- Local state that mirrors remote data and can get out of sync
- Recommendation: derive from the query result directly; avoid duplicating server state locally
5. Testing Patterns (React/TSX)
When reviewing *.test.tsx changes, apply the React testing rules in .claude/rules/testing/react.md in addition to the general FXA rules in .claude/rules/testing/base.md. Both rules auto-load when Claude reads a matching test file; this skill's pre-flight should explicitly Read them too so reviews cover test changes consistently. Headline triggers to flag: implementation-detail queries (className/CSS/data-testid where semantic queries fit), fireEvent where userEvent belongs, asserting on instances/refs/private state, missing or misused act(), whole-tree snapshots for non-trivial components, and duplicated provider boilerplate.
Output Format
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.
