React Agents Review

react-agents-review

Quick Reference

This skill provides a structured validation checklist for reviewing generated React code. Run each checklist area sequentially against the code under review. Every item uses the format:

• [CHECK] What to verify
• [PASS] Expected correct state
• [FAIL] Common failure mode and fix

Critical Warnings

NEVER approve code with conditional Hook calls -- this breaks React's internal state tracking and causes silent corruption across re-renders.

NEVER approve useEffect without verifying its cleanup function -- missing cleanup causes memory leaks, stale subscriptions, and zombie event listeners.

NEVER approve dangerouslySetInnerHTML without DOMPurify or equivalent sanitization -- this is a direct XSS attack vector.

NEVER approve components over 300 lines without flagging for decomposition -- God components are untestable and unmaintainable.

ALWAYS verify TypeScript types are explicit on all props interfaces -- any types defeat the purpose of TypeScript and hide bugs.

---

Checklist 1: Rules of Hooks

• [CHECK] All Hook calls are at the top level of the component or custom Hook
• [PASS] Hooks appear before any return, if, for, while, or switch statement
• [FAIL] Hook called inside a condition, loop, or nested function -- MOVE it to top level and guard the logic inside the Hook instead

• [CHECK] Hooks are ONLY called from React function components or custom Hooks
• [PASS] Calling function starts with uppercase (component) or use (custom Hook)
• [FAIL] Hook called from a plain utility function -- EXTRACT into a custom Hook prefixed with use

• [CHECK] useEffect dependency arrays are complete
• [PASS] Every variable referenced inside the effect callback appears in the dependency array
• [FAIL] Missing dependency causes stale closures -- ADD the missing dependency or wrap the value in useRef if intentionally stable

• [CHECK] useMemo/useCallback dependency arrays match usage
• [PASS] All referenced values in the memoized computation are listed as dependencies
• [FAIL] Stale memoized value returned -- ADD missing dependencies

• [CHECK] Custom Hooks follow naming convention
• [PASS] Custom Hook name starts with use followed by an uppercase letter (e.g., useFormState)
• [FAIL] Hook not recognized by React linter -- RENAME to start with use

---

Checklist 2: TypeScript Type Safety

• [CHECK] Component props use an explicit interface or type
• [PASS] interface Props { ... } defined and applied: function Component(props: Props) or destructured
• [FAIL] Props typed as any, object, or not typed -- DEFINE an explicit interface

• [CHECK] Event handlers use correct React event types
• [PASS] React.MouseEvent<HTMLButtonElement>, React.ChangeEvent<HTMLInputElement>, React.FormEvent<HTMLFormElement>
• [FAIL] Event typed as any or generic Event -- USE the specific React.*Event<HTMLElement> type

• [CHECK] Refs are correctly typed with useRef
• [PASS] useRef<HTMLDivElement>(null) with initial null for DOM refs; useRef<number>(0) for mutable values
• [FAIL] useRef<HTMLElement>() without null initial value -- the .current type excludes null, causing runtime errors on first render

• [CHECK] State is typed explicitly when not inferable
• [PASS] useState<User | null>(null) when initial value does not reflect full type range
• [FAIL] useState(null) without generic -- TypeScript infers null only, blocking later assignments

• [CHECK] createContext uses a generic type
• [PASS] createContext<ThemeContextType | null>(null) with a defined context type
• [FAIL] createContext({}) with no type -- consumers receive {} type, no autocompletion or safety

• [CHECK] Children prop uses correct type
• [PASS] React.ReactNode for general children; React.ReactElement when exactly one element required
• [FAIL] children: JSX.Element -- does not accept strings, numbers, fragments, or arrays

---

Checklist 3: Performance

• [CHECK] Components receiving non-primitive props are wrapped with React.memo where appropriate
• [PASS] React.memo(Component) applied to components that re-render with unchanged props due to parent re-renders
• [FAIL] Component re-renders on every parent render despite stable props -- WRAP with React.memo

• [CHECK] React.memo is NOT applied prematurely
• [PASS] React.memo used only when profiling confirms unnecessary re-renders
• [FAIL] Every component wrapped in React.memo -- REMOVE from simple components where the comparison cost exceeds re-render cost

• [CHECK] Object/array/function props are referentially stable
• [PASS] Objects created via useMemo, functions via useCallback, or defined outside the component
• [FAIL] Inline {{ color: 'red' }} or () => handleClick(id) in JSX -- EXTRACT with useMemo/useCallback or move to module scope

• [CHECK] React Compiler readiness (React 19)
• [PASS] No manual useMemo/useCallback that the compiler would auto-generate; pure component logic
• [FAIL] Complex memoization chains that fight the compiler -- SIMPLIFY and let the compiler optimize

• [CHECK] Expensive computations are memoized
• [PASS] useMemo(() => expensiveFilter(items), [items]) for O(n) or worse operations
• [FAIL] Filtering/sorting a large array on every render -- WRAP in useMemo with correct dependencies

• [CHECK] Lists use stable, unique keys
• [PASS] key={item.id} using a unique identifier from the data
• [FAIL] key={index} on a list that can reorder, filter, or insert -- USE a stable unique ID

---

Checklist 4: State Management

• [CHECK] No derived state stored in useState
• [PASS] Derived values computed directly in the render body: const fullName = first + ' ' + last
• [FAIL] useEffect syncing derived state from props/other state -- REMOVE the state and compute inline

• [CHECK] No state duplication
• [PASS] Single source of truth for each piece of data
• [FAIL] Same data stored in multiple useState calls that must be kept in sync -- CONSOLIDATE into one state or derive

• [CHECK] Appropriate state colocation
• [PASS] State lives in the lowest common ancestor that needs it
• [FAIL] State lifted to app root when only two sibling components need it -- MOVE state down

• [CHECK] Context vs prop drilling vs external store is appropriate
• [PASS] Props for 1-2 levels; Context for widely-shared low-frequency data; external store (Zustand/Redux) for high-frequency cross-cutting state
• [FAIL] Context used for rapidly changing values (mouse position, animation frames) -- USE an external store with selectors

• [CHECK] useReducer used for complex state logic
• [PASS] Related state transitions grouped in a reducer when 3+ useState calls interact
• [FAIL] Multiple useState calls updated together in event handlers -- CONSOLIDATE into useReducer

---

Checklist 5: Event Handling and Effects

• [CHECK] useEffect includes cleanup for subscriptions and listeners
• [PASS] Returns cleanup function: return () => { subscription.unsubscribe(); }
• [FAIL] addEventListener without corresponding removeEventListener in cleanup -- ADD cleanup return

• [CHECK] No stale closures in event handlers
• [PASS] Event handlers reference current state via useCallback with correct deps, or use functional state updates
• [FAIL] Handler captures initial state value and never updates -- USE setState(prev => ...) or add to useCallback deps

• [CHECK] Async effects handle component unmount
• [PASS] Uses abort controller or boolean flag: const aborted = { current: false }; return () => { aborted.current = true; }
• [FAIL] setState called after unmount in an async callback -- ADD abort/cancelled check

• [CHECK] useEffect is not used for event handlers
• [PASS] Click/submit handlers attached directly via JSX onClick/onSubmit
• [FAIL] useEffect with state dependency that triggers an action -- MOVE logic to the event handler that changes the state

---

Checklist 6: Forms

• [CHECK] Controlled vs uncontrolled pattern is consistent
• [PASS] Controlled: value={state} + onChange; Uncontrolled: defaultValue + ref
• [FAIL] Mixing value and defaultValue or switching between controlled/uncontrolled -- CHOOSE one pattern

• [CHECK] Form validation provides user feedback
• [PASS] Validation errors displayed per-field with aria-describedby linking error to input
• [FAIL] Silent validation failure or only console.log -- ADD visible error messages

• [CHECK] React 19 form actions used where appropriate
• [PASS] <form action={submitAction}> with useActionState for server-side form handling
• [FAIL] Manual onSubmit + preventDefault + fetch in React 19 -- CONSIDER useActionState for simpler data flow

---

Checklist 7: Component Patterns

• [CHECK] Component is under 300 lines
• [PASS] Focused, single-responsibility component
• [FAIL] God component with multiple concerns -- SPLIT into smaller components using composition

• [CHECK] Composition over configuration
• [PASS] Uses children, render props, or slots for flexible content injection
• [FAIL] Massive prop interface with 10+ boolean flags controlling rendering -- REFACTOR to composition pattern

• [CHECK] Key prop usage is correct
• [PASS] Keys on the outermost element in a .map() call; keys are stable and unique among siblings
• [FAIL] Key placed on an inner element, or key is Math.random() -- MOVE key to outermost mapped element and use stable ID

---

Checklist 8: Server Components (React 19 / Next.js App Router)

• [CHECK] No client-only code in Server Components
• [PASS] Server Components contain NO useState, useEffect, useRef, event handlers, or browser APIs
• [FAIL] Hook or window reference in a Server Component -- ADD 'use client' directive or extract to a Client Component

• [CHECK] Props passed from Server to Client Components are serializable
• [PASS] Props are plain objects, arrays, strings, numbers, booleans, or null
• [FAIL] Functions, classes, Date objects, or Map/Set passed as props -- SERIALIZE or restructure

• [CHECK] 'use client' boundary is intentional and minimal
• [PASS] 'use client' placed at the lowest component that needs interactivity; Server Components compose above it
• [FAIL] 'use client' at the page level, converting the entire tree to client -- PUSH the boundary down

• [CHECK] 'use server' functions validate input
• [PASS] Server Actions validate and sanitize all parameters before use
• [FAIL] Server Action trusts client input directly -- ADD validation with zod or similar

---

Checklist 9: Accessibility

• [CHECK] Interactive elements have accessible names
• [PASS] Buttons have text content or aria-label; inputs have associated <label> or aria-label
• [FAIL] Icon-only button with no accessible name -- ADD aria-label describing the action

• [CHECK] Semantic HTML is used
• [PASS] <button> for actions, <a> for navigation, <nav>, <main>, <header>, <section> for structure
• [FAIL] <div onClick> instead of <button> -- REPLACE with the semantic element

• [CHECK] Keyboard navigation works
• [PASS] All interactive elements are focusable and operable with keyboard; custom widgets implement ARIA keyboard patterns
• [FAIL] Custom dropdown only works with mouse -- ADD onKeyDown handling for Arrow/Enter/Escape keys

• [CHECK] Dynamic content updates are announced
• [PASS] aria-live="polite" on regions that update asynchronously (toast messages, loading states)
• [FAIL] Content appears/disappears with no screen reader announcement -- ADD aria-live region

---

Checklist 10: Security

• [CHECK] dangerouslySetInnerHTML content is sanitized
• [PASS] Input passed through DOMPurify: dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(html) }}
• [FAIL] Raw user input in dangerouslySetInnerHTML -- ADD DOMPurify.sanitize() or remove

• [CHECK] User input is not interpolated into href without validation
• [PASS] URLs validated against allowlist or parsed with new URL() to block javascript: protocol
• [FAIL] <a href={userInput}> without validation -- ADD URL validation

• [CHECK] No sensitive data in client-side state
• [PASS] Tokens stored in httpOnly cookies; secrets stay server-side
• [FAIL] API keys or tokens in useState or localStorage accessible to XSS -- MOVE to server-side session

---

Checklist 11: Testing

• [CHECK] Tests query by role, label, or text -- NOT by implementation details
• [PASS] screen.getByRole('button', { name: /submit/i }), screen.getByLabelText('Email')
• [FAIL] container.querySelector('.btn-primary') or getByTestId as first choice -- USE role-based queries

• [CHECK] Error and loading states are tested
• [PASS] Tests cover: initial render, loading state, success state, error state
• [FAIL] Only happy-path test -- ADD tests for loading and error scenarios

• [CHECK] User interactions use userEvent over fireEvent
• [PASS] await userEvent.click(button) for realistic event simulation
• [FAIL] fireEvent.click(button) for user-driven interactions -- USE @testing-library/user-event

• [CHECK] Async operations are properly awaited
• [PASS] await screen.findByText('loaded') or await waitFor(() => expect(...)) for async state updates
• [FAIL] Assertion runs before async update completes -- USE findBy queries or waitFor

• [CHECK] No Enzyme usage in new code
• [PASS] React Testing Library used for all component tests
• [FAIL] shallow() or mount() from Enzyme -- MIGRATE to React Testing Library

---

Review Execution Order

When reviewing React code, ALWAYS run checklists in this order:

1. Rules of Hooks (Checklist 1) -- incorrect Hook usage breaks the entire component 2. TypeScript (Checklist 2) -- type errors propagate through the component tree 3. State Management (Checklist 4) -- wrong state architecture causes cascading issues 4. Event Handling (Checklist 5) -- memory leaks and stale closures are silent bugs 5. Server Components (Checklist 8) -- boundary errors crash at runtime 6. Security (Checklist 10) -- XSS vulnerabilities must be caught early 7. Performance (Checklist 3) -- optimize after correctness 8. Forms (Checklist 6) -- validation and pattern consistency 9. Component Patterns (Checklist 7) -- structural improvements 10. Accessibility (Checklist 9) -- ensure inclusive design 11. Testing (Checklist 11) -- verify test quality

---

Reference Links

• references/examples.md -- Review scenarios with good code, bad code, and fixes
• references/anti-patterns.md -- All anti-patterns consolidated across all checklist areas

Official Sources

• https://react.dev/reference/rules/rules-of-hooks
• https://react.dev/learn/you-might-not-need-an-effect
• https://react.dev/reference/react/memo
• https://react.dev/reference/react/useEffect
• https://react.dev/learn/thinking-in-react
• https://react.dev/reference/rsc/server-components
• https://react.dev/learn/typescript
• https://testing-library.com/docs/react-testing-library/intro

react-agents-review: Anti-Patterns Reference

Consolidated list of all React anti-patterns organized by category. Each entry includes the anti-pattern, why it is wrong, and the correct approach.

---

Rules of Hooks Anti-Patterns

AP-H01: Conditional Hook Call

// WRONG
if (isLoggedIn) {
  const [user, setUser] = useState(null);
}

Why: React tracks Hooks by call order. Conditional calls change the order between renders, corrupting all subsequent Hook state.

Fix: ALWAYS call Hooks unconditionally at the top level. Move the condition inside the Hook's logic.

AP-H02: Hook in Loop

// WRONG
for (const id of itemIds) {
  const [data, setData] = useState(null);
}

Why: The number of Hook calls changes with array length, breaking React's internal mapping.

Fix: ALWAYS use a single useState with an object or array, or extract each item into a child component with its own state.

AP-H03: Hook in Nested Function

// WRONG
function Component() {
  const handleClick = () => {
    const [count, setCount] = useState(0); // Hook inside callback
  };
}

Why: Hooks must be called during the render phase. Callbacks execute after render.

Fix: ALWAYS place Hook calls at the top level of the component body.

AP-H04: Missing useEffect Dependencies

// WRONG
useEffect(() => {
  fetchData(userId);
}, []); // userId not in deps

Why: Effect uses userId but does not re-run when it changes -- shows stale data.

Fix: ALWAYS include all referenced variables in the dependency array: [userId].

AP-H05: Object in useEffect Dependencies

// WRONG
useEffect(() => {
  doSomething(options);
}, [options]); // options is { page: 1 } created each render

Why: A new object reference is created every render, so the effect runs every render despite identical values.

Fix: Destructure to primitives in deps: [options.page], or stabilize with useMemo.

---

TypeScript Anti-Patterns

AP-T01: Props Typed as any

// WRONG
function Button(props: any) { ... }

Why: Defeats TypeScript's purpose. No autocomplete, no compile-time error detection.

Fix: ALWAYS define an explicit interface for props.

AP-T02: Untyped Event Handlers

// WRONG
const handleChange = (e: any) => { ... };

Why: Loses type safety for e.target.value, e.currentTarget, etc.

Fix: (e: React.ChangeEvent<HTMLInputElement>) => { ... }

AP-T03: useRef Without null Initial Value

// WRONG (for DOM refs)
const ref = useRef<HTMLDivElement>();

Why: TypeScript infers MutableRefObject<HTMLDivElement | undefined>. React assigns null initially, not undefined. The type is wrong.

Fix: useRef<HTMLDivElement>(null) -- produces RefObject<HTMLDivElement> with correct null handling.

AP-T04: useState Without Generic for Nullable

// WRONG
const [user, setUser] = useState(null);

Why: TypeScript infers useState<null> -- you cannot later call setUser(userData).

Fix: useState<User | null>(null)

AP-T05: Children Typed as JSX.Element

// WRONG
interface Props { children: JSX.Element; }

Why: Rejects strings, numbers, arrays, fragments, and null.

Fix: Use React.ReactNode for general children content.

---

Performance Anti-Patterns

AP-P01: Inline Object Props

// WRONG
<Component style={{ color: 'red' }} config={{ theme: 'dark' }} />

Why: New object reference on every render defeats React.memo and causes unnecessary child re-renders.

Fix: Define outside component or stabilize with useMemo.

AP-P02: Inline Function Props

// WRONG
<Button onClick={() => handleClick(id)} />

Why: New function reference every render. If Button is memoized, the memo is useless.

Fix: useCallback with stable deps, or restructure so the child component receives the ID as a prop and handles the call.

AP-P03: Memo on Every Component

// WRONG
const SimpleText = React.memo(({ text }: { text: string }) => <span>{text}</span>);

Why: For trivial components, the comparison cost exceeds the re-render cost. Adds code complexity for negative performance gain.

Fix: ONLY use React.memo when profiling confirms unnecessary re-renders on components with expensive render logic or many children.

AP-P04: Index as Key in Dynamic Lists

// WRONG
{items.map((item, index) => <Item key={index} data={item} />)}

Why: When items reorder, insert, or delete, React matches by key. Index keys cause it to update the wrong DOM elements, leading to visual glitches and lost component state.

Fix: ALWAYS use key={item.id} with a unique, stable identifier.

AP-P05: Unmemoized Expensive Computation

// WRONG
function ProductList({ products, filter }: Props) {
  const filtered = products.filter(p => matchesFilter(p, filter)); // runs every render
}

Why: If products has thousands of entries, filtering runs on every keystroke or unrelated state change.

Fix: useMemo(() => products.filter(...), [products, filter])

---

State Management Anti-Patterns

AP-S01: Derived State in useState

// WRONG
const [fullName, setFullName] = useState('');
useEffect(() => {
  setFullName(`${firstName} ${lastName}`);
}, [firstName, lastName]);

Why: Causes double render. The value is deterministic from existing state -- it does not need its own state.

Fix: const fullName = ${firstName} ${lastName}; -- compute during render.

AP-S02: Props Copied to State

// WRONG
function Child({ initialValue }: { initialValue: string }) {
  const [value, setValue] = useState(initialValue);
  // value is now disconnected from parent -- updates to initialValue are lost
}

Why: State initialized from props creates a copy. Parent prop changes are silently ignored.

Fix: If the value should stay in sync with the parent, use the prop directly. If it should be independent, name the prop initialValue and document that it only seeds initial state.

AP-S03: State Duplication

// WRONG
const [items, setItems] = useState<Item[]>([]);
const [itemCount, setItemCount] = useState(0);
// Must manually keep itemCount in sync with items.length

Why: Two sources of truth that can diverge.

Fix: const itemCount = items.length; -- derive, do not duplicate.

AP-S04: Context for High-Frequency Updates

// WRONG
const MouseContext = createContext({ x: 0, y: 0 });
// Every mousemove re-renders ALL consumers

Why: Context has no selector mechanism. Every consumer re-renders when any part of the context value changes.

Fix: Use an external store (Zustand, Jotai) with selectors for high-frequency data.

AP-S05: Too Many Related useState Calls

// WRONG
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<Error | null>(null);
const [data, setData] = useState<Data | null>(null);
// Must coordinate three state updates in every handler

Why: Related state updated separately can produce impossible states (e.g., isLoading: true AND error: not null).

Fix: Use useReducer to manage related state transitions atomically.

---

Event Handling Anti-Patterns

AP-E01: Missing Effect Cleanup

// WRONG
useEffect(() => {
  window.addEventListener('scroll', handleScroll);
  // no return () => removeEventListener
}, []);

Why: Listener accumulates on every mount. In Strict Mode (dev), mounts twice immediately.

Fix: ALWAYS return a cleanup function from effects that add listeners or subscriptions.

AP-E02: Stale Closure

// WRONG
const [count, setCount] = useState(0);
const handleClick = useCallback(() => {
  console.log(count); // always logs 0
}, []); // count missing from deps

Why: handleClick closes over the initial count value and never updates.

Fix: Add count to the dependency array, or use setCount(prev => prev + 1) for state updates.

AP-E03: useEffect for Event Handler Logic

// WRONG
const [query, setQuery] = useState('');
useEffect(() => {
  if (query) {
    search(query); // triggered by effect, not by the event
  }
}, [query]);

Why: The search is a response to a user action (typing), not a synchronization need. This pattern obscures the data flow and makes it hard to add debouncing or cancel logic.

Fix: Call search(query) directly in the onChange handler (with debounce if needed).

AP-E04: Async setState After Unmount

// WRONG
useEffect(() => {
  fetchData().then(data => setData(data)); // may run after unmount
}, []);

Why: If the component unmounts before the fetch resolves, setData is called on an unmounted component.

Fix: Use AbortController to cancel the request on cleanup.

---

Server Component Anti-Patterns

AP-SC01: Hooks in Server Components

// WRONG -- Server Component file (no 'use client')
import { useState } from 'react';
export default function Page() {
  const [count, setCount] = useState(0); // Runtime error
}

Why: Server Components execute on the server and stream HTML. They cannot maintain client-side state.

Fix: Add 'use client' at the top of the file, or extract the interactive part into a separate Client Component.

AP-SC02: Non-Serializable Props to Client Components

// WRONG
<ClientComponent onSubmit={handleSubmit} createdAt={new Date()} />

Why: Functions and Date objects cannot cross the server-client boundary. Props are serialized to JSON.

Fix: Pass serializable data only. Use Server Actions ('use server') for function-like behavior. Pass ISO strings for dates.

AP-SC03: use client at Page Level

// WRONG
'use client'; // Entire page is now a Client Component
export default function DashboardPage() { ... }

Why: Makes the entire page tree client-rendered, losing all Server Component benefits (smaller bundle, direct data access, streaming).

Fix: PUSH 'use client' to the smallest interactive leaf components.

AP-SC04: Unvalidated Server Actions

// WRONG
'use server';
async function deleteUser(userId: string) {
  await db.users.delete(userId); // trusts client input
}

Why: Server Actions are public HTTP endpoints. Any client can call them with arbitrary arguments.

Fix: ALWAYS validate input (zod, etc.) and check authorization before performing mutations.

---

Accessibility Anti-Patterns

AP-A01: Clickable Div

// WRONG
<div onClick={handleClick}>Click me</div>

Why: Not focusable, no keyboard support, not announced as interactive by screen readers.

Fix: Use <button> for actions, <a> for navigation.

AP-A02: Missing Alt Text

// WRONG
<img src={photo.url} />

Why: Screen readers announce the file name or nothing. Image content is inaccessible.

Fix: ALWAYS provide descriptive alt text. Use alt="" only for purely decorative images.

AP-A03: Icon-Only Button Without Label

// WRONG
<button><TrashIcon /></button>

Why: Screen reader announces "button" with no description of what it does.

Fix: <button aria-label="Delete item"><TrashIcon /></button>

AP-A04: Missing Form Labels

// WRONG
<input type="email" placeholder="Email" />

Why: Placeholder is not a label. It disappears on focus and is not reliably announced.

Fix: <label htmlFor="email">Email</label><input id="email" type="email" />

AP-A05: No Live Region for Dynamic Content

// WRONG
{isLoading && <div>Loading...</div>}
{error && <div className="error">{error}</div>}

Why: Screen readers do not announce dynamically appearing content by default.

Fix: Wrap in <div role="status" aria-live="polite"> for loading states, <div role="alert"> for errors.

---

Security Anti-Patterns

AP-SEC01: Unsanitized dangerouslySetInnerHTML

// WRONG
<div dangerouslySetInnerHTML={{ __html: userInput }} />

Why: Direct XSS attack vector. User can inject <script> tags or event handlers.

Fix: dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(userInput) }}

AP-SEC02: Unvalidated href

// WRONG
<a href={userProvidedUrl}>Visit</a>

Why: User can provide javascript:alert('xss') as the URL.

Fix: Validate URL scheme: if (url.startsWith('https://') || url.startsWith('http://')).

AP-SEC03: Secrets in Client State

// WRONG
const [apiKey] = useState(process.env.NEXT_PUBLIC_API_SECRET);

Why: NEXT_PUBLIC_* variables are embedded in the client bundle. Anyone can read them.

Fix: Keep secrets server-side. Use API routes or Server Actions to proxy authenticated requests.

---

Testing Anti-Patterns

AP-TEST01: Implementation Detail Queries

// WRONG
container.querySelector('.btn-primary');
wrapper.find('Button').prop('onClick');

Why: Coupled to CSS classes and component structure. Any refactor breaks the test without changing behavior.

Fix: screen.getByRole('button', { name: /submit/i })

AP-TEST02: fireEvent Instead of userEvent

// WRONG
fireEvent.change(input, { target: { value: 'text' } });

Why: fireEvent dispatches a single DOM event. Real users trigger focus, keydown, input, change, blur. fireEvent misses validation and interaction bugs.

Fix: await userEvent.type(input, 'text')

AP-TEST03: No Error State Tests

// WRONG -- only tests happy path
test('shows data', async () => {
  mockApi.get.mockResolvedValue(data);
  render(<DataView />);
  expect(await screen.findByText('result')).toBeInTheDocument();
});

Why: Error and loading states are user-facing UI. Untested states break silently.

Fix: Add tests for loading state, error state, empty state, and edge cases.

AP-TEST04: Enzyme Usage

// WRONG
import { shallow } from 'enzyme';
const wrapper = shallow(<Component />);

Why: Enzyme is unmaintained and does not support React 18+. shallow rendering tests implementation details.

Fix: ALWAYS use React Testing Library for new tests. Migrate existing Enzyme tests.

AP-TEST05: Missing Async Handling

// WRONG
render(<AsyncComponent />);
expect(screen.getByText('loaded')).toBeInTheDocument(); // fails -- not yet loaded

Why: Assert runs synchronously before the async state update.

Fix: expect(await screen.findByText('loaded')).toBeInTheDocument()

react-agents-review: Review Scenarios

Scenario 1: Rules of Hooks Violation

Bad Code

function UserProfile({ userId }: { userId: string | null }) {
  if (!userId) return <div>No user selected</div>;

  // VIOLATION: Hook called after conditional return
  const [user, setUser] = useState<User | null>(null);

  useEffect(() => {
    fetchUser(userId).then(setUser);
  }, [userId]);

  return <div>{user?.name}</div>;
}

What Fails

• Checklist 1, Item 1: Hook call is NOT at the top level -- it appears after a conditional return
• React tracks Hooks by call order; early return changes the Hook count between renders

Fixed Code

function UserProfile({ userId }: { userId: string | null }) {
  const [user, setUser] = useState<User | null>(null);

  useEffect(() => {
    if (!userId) return;
    fetchUser(userId).then(setUser);
  }, [userId]);

  if (!userId) return <div>No user selected</div>;

  return <div>{user?.name}</div>;
}

---

Scenario 2: Missing Effect Cleanup

Bad Code

function WindowSize() {
  const [width, setWidth] = useState(window.innerWidth);

  useEffect(() => {
    const handleResize = () => setWidth(window.innerWidth);
    window.addEventListener('resize', handleResize);
    // NO cleanup function returned
  }, []);

  return <span>{width}px</span>;
}

What Fails

• Checklist 5, Item 1: addEventListener without corresponding removeEventListener in cleanup
• Every mount adds a new listener; unmount never removes it -- memory leak

Fixed Code

function WindowSize() {
  const [width, setWidth] = useState(window.innerWidth);

  useEffect(() => {
    const handleResize = () => setWidth(window.innerWidth);
    window.addEventListener('resize', handleResize);
    return () => window.removeEventListener('resize', handleResize);
  }, []);

  return <span>{width}px</span>;
}

---

Scenario 3: Derived State Anti-Pattern

Bad Code

interface Props {
  items: Item[];
  searchTerm: string;
}

function ItemList({ items, searchTerm }: Props) {
  const [filteredItems, setFilteredItems] = useState<Item[]>([]);

  // ANTI-PATTERN: syncing derived state via useEffect
  useEffect(() => {
    setFilteredItems(items.filter(item =>
      item.name.toLowerCase().includes(searchTerm.toLowerCase())
    ));
  }, [items, searchTerm]);

  return (
    <ul>
      {filteredItems.map(item => (
        <li key={item.id}>{item.name}</li>
      ))}
    </ul>
  );
}

What Fails

• Checklist 4, Item 1: Derived state stored in useState and synced via useEffect
• Causes an unnecessary extra render cycle: props change -> render with stale filter -> effect runs -> re-render with correct filter

Fixed Code

interface Props {
  items: Item[];
  searchTerm: string;
}

function ItemList({ items, searchTerm }: Props) {
  // Compute directly during render -- no state needed
  const filteredItems = useMemo(
    () => items.filter(item =>
      item.name.toLowerCase().includes(searchTerm.toLowerCase())
    ),
    [items, searchTerm]
  );

  return (
    <ul>
      {filteredItems.map(item => (
        <li key={item.id}>{item.name}</li>
      ))}
    </ul>
  );
}

---

Scenario 4: TypeScript Type Safety Gaps

Bad Code

function Form({ onSubmit }: any) {
  const inputRef = useRef<HTMLInputElement>();

  const handleSubmit = (e: any) => {
    e.preventDefault();
    onSubmit(inputRef.current.value); // potential null access
  };

  return (
    <form onSubmit={handleSubmit}>
      <input ref={inputRef} />
      <button type="submit">Submit</button>
    </form>
  );
}

What Fails

• Checklist 2, Item 1: Props typed as any
• Checklist 2, Item 2: Event typed as any instead of React.FormEvent<HTMLFormElement>
• Checklist 2, Item 3: useRef without null initial value -- .current type does not include null, but the actual value is null on first render

Fixed Code

interface FormProps {
  onSubmit: (value: string) => void;
}

function Form({ onSubmit }: FormProps) {
  const inputRef = useRef<HTMLInputElement>(null);

  const handleSubmit = (e: React.FormEvent<HTMLFormElement>) => {
    e.preventDefault();
    if (inputRef.current) {
      onSubmit(inputRef.current.value);
    }
  };

  return (
    <form onSubmit={handleSubmit}>
      <input ref={inputRef} />
      <button type="submit">Submit</button>
    </form>
  );
}

---

Scenario 5: Server Component Boundary Error

Bad Code

// app/dashboard/page.tsx (Server Component by default in Next.js App Router)
import { useState } from 'react';

export default function DashboardPage() {
  const [tab, setTab] = useState('overview'); // ERROR: Hook in Server Component

  return (
    <div>
      <button onClick={() => setTab('overview')}>Overview</button>
      <button onClick={() => setTab('stats')}>Stats</button>
      {tab === 'overview' ? <Overview /> : <Stats />}
    </div>
  );
}

What Fails

• Checklist 8, Item 1: useState and event handlers in a Server Component
• Server Components cannot use Hooks or attach event listeners -- they render on the server only

Fixed Code

// app/dashboard/page.tsx (Server Component -- fetches data)
import { TabNavigation } from './tab-navigation';
import { getOverviewData, getStatsData } from '@/lib/data';

export default async function DashboardPage() {
  const [overviewData, statsData] = await Promise.all([
    getOverviewData(),
    getStatsData(),
  ]);

  return (
    <TabNavigation
      overviewData={overviewData}
      statsData={statsData}
    />
  );
}

// app/dashboard/tab-navigation.tsx (Client Component -- handles interactivity)
'use client';

import { useState } from 'react';

interface TabNavigationProps {
  overviewData: OverviewData;
  statsData: StatsData;
}

export function TabNavigation({ overviewData, statsData }: TabNavigationProps) {
  const [tab, setTab] = useState<'overview' | 'stats'>('overview');

  return (
    <div>
      <button onClick={() => setTab('overview')}>Overview</button>
      <button onClick={() => setTab('stats')}>Stats</button>
      {tab === 'overview' ? (
        <Overview data={overviewData} />
      ) : (
        <Stats data={statsData} />
      )}
    </div>
  );
}

---

Scenario 6: Accessibility Failures

Bad Code

function ImageGallery({ images }: { images: Image[] }) {
  const [selected, setSelected] = useState(0);

  return (
    <div>
      {images.map((img, i) => (
        <div
          key={img.id}
          onClick={() => setSelected(i)}
          style={{ border: i === selected ? '2px solid blue' : 'none' }}
        >
          <img src={img.url} />
        </div>
      ))}
    </div>
  );
}

What Fails

• Checklist 9, Item 1: No accessible name on the clickable div
• Checklist 9, Item 2: <div onClick> instead of <button> for interactive element
• Checklist 9, Item 3: No keyboard navigation -- <div> is not focusable
• Missing alt attribute on <img>

Fixed Code

function ImageGallery({ images }: { images: Image[] }) {
  const [selected, setSelected] = useState(0);

  return (
    <div role="listbox" aria-label="Image gallery">
      {images.map((img, i) => (
        <button
          key={img.id}
          role="option"
          aria-selected={i === selected}
          onClick={() => setSelected(i)}
          style={{
            border: i === selected ? '2px solid blue' : 'none',
            background: 'none',
            padding: 0,
            cursor: 'pointer',
          }}
        >
          <img src={img.url} alt={img.description} />
        </button>
      ))}
    </div>
  );
}

---

Scenario 7: Testing Anti-Patterns

Bad Code

import { render } from '@testing-library/react';
import { fireEvent } from '@testing-library/react';

test('counter increments', () => {
  const { container } = render(<Counter />);
  const button = container.querySelector('.increment-btn');
  fireEvent.click(button!);
  const display = container.querySelector('.count-display');
  expect(display?.textContent).toBe('1');
});

What Fails

• Checklist 11, Item 1: Queries use CSS selectors (.increment-btn, .count-display) instead of roles or text
• Checklist 11, Item 3: fireEvent used instead of userEvent for user-driven interaction
• Test is coupled to class names -- refactoring CSS breaks the test

Fixed Code

import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

test('counter increments when increment button is clicked', async () => {
  render(<Counter />);

  const button = screen.getByRole('button', { name: /increment/i });
  await userEvent.click(button);

  expect(screen.getByText('1')).toBeInTheDocument();
});

---

Scenario 8: Stale Closure in Async Effect

Bad Code

function SearchResults({ query }: { query: string }) {
  const [results, setResults] = useState<Result[]>([]);

  useEffect(() => {
    fetch(`/api/search?q=${query}`)
      .then(res => res.json())
      .then(data => setResults(data));
  }, [query]);

  return <ResultList results={results} />;
}

What Fails

• Checklist 5, Item 3: No abort handling -- if query changes rapidly, responses arrive out of order and the UI shows results for a stale query
• Race condition: fast typing sends multiple requests; last response wins regardless of query order

Fixed Code

function SearchResults({ query }: { query: string }) {
  const [results, setResults] = useState<Result[]>([]);

  useEffect(() => {
    const controller = new AbortController();

    fetch(`/api/search?q=${query}`, { signal: controller.signal })
      .then(res => res.json())
      .then(data => setResults(data))
      .catch(err => {
        if (err.name !== 'AbortError') throw err;
      });

    return () => controller.abort();
  }, [query]);

  return <ResultList results={results} />;
}

---

Scenario 9: Premature Optimization

Bad Code

const Label = React.memo(({ text }: { text: string }) => {
  return <span>{text}</span>;
});

const Divider = React.memo(() => {
  return <hr />;
});

const Page = React.memo(({ title }: { title: string }) => {
  return (
    <div>
      <Label text={title} />
      <Divider />
    </div>
  );
});

What Fails

• Checklist 3, Item 2: React.memo applied to trivial components
• Label receives a primitive string -- comparison cost is comparable to re-render cost
• Divider has no props -- React.memo comparison is pure overhead
• The shallow comparison on every render likely costs MORE than just re-rendering these tiny components

Fixed Code

function Label({ text }: { text: string }) {
  return <span>{text}</span>;
}

function Divider() {
  return <hr />;
}

function Page({ title }: { title: string }) {
  return (
    <div>
      <Label text={title} />
      <Divider />
    </div>
  );
}

Reserve React.memo for components where profiling shows measurable unnecessary re-renders with complex prop trees or expensive render logic.