fxa-check-react

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.