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.