references/antipatterns.md
# React + TypeScript Anti-Patterns Catalog
## useEffect Anti-Patterns
### 1. Derived State in useEffect
**Problem:** Extra render cycle, state synchronization bugs.
```typescript
// ❌ ANTI-PATTERN
const [firstName, setFirstName] = useState('Taylor');
const [lastName, setLastName] = useState('Swift');
const [fullName, setFullName] = useState('');
useEffect(() => {
setFullName(firstName + ' ' + lastName); // Extra render!
}, [firstName, lastName]);
// ✅ CORRECT: Calculate during render
const fullName = firstName + ' ' + lastName;
```
**Detection:** `useEffect` that only calls `setState` with computed values from dependencies.
### 2. Event Logic in useEffect
**Problem:** Side effect runs on mount/remount, not just on user action.
```typescript
// ❌ ANTI-PATTERN: Notification shows on page reload too
useEffect(() => {
if (product.isInCart) {
showNotification(`Added ${product.name}!`);
}
}, [product]);
// ✅ CORRECT: Logic in event handler
function handleBuyClick() {
addToCart(product);
showNotification(`Added ${product.name}!`);
}
```
**Detection:** `useEffect` containing UI feedback (toast, modal, notification) triggered by state change.
### 3. Resetting State on Prop Change
**Problem:** Extra render, harder to trace state flow.
```typescript
// ❌ ANTI-PATTERN
function ProfilePage({ userId }: { userId: string }) {
const [comment, setComment] = useState('');
useEffect(() => {
setComment(''); // Reset on user change
}, [userId]);
}
// ✅ CORRECT: Use key to reset
function ProfilePage({ userId }: { userId: string }) {
return <Profile userId={userId} key={userId} />;
}
```
**Detection:** `useEffect` that resets state when a prop changes.
### 4. Fetching Without Proper Cleanup
**Problem:** Race conditions, memory leaks, setting state on unmounted component.
```typescript
// ❌ ANTI-PATTERN: No cleanup
useEffect(() => {
fetch(`/api/user/${userId}`)
.then(res => res.json())
.then(data => setUser(data)); // May set state after unmount
}, [userId]);
// ✅ CORRECT: AbortController for cleanup
useEffect(() => {
const controller = new AbortController();
fetch(`/api/user/${userId}`, { signal: controller.signal })
.then(res => res.json())
.then(data => setUser(data))
.catch(err => {
if (err.name !== 'AbortError') setError(err);
});
return () => controller.abort();
}, [userId]);
// ✅ BETTER: Use TanStack Query
const { data: user } = useQuery({
queryKey: ['user', userId],
queryFn: () => fetch(`/api/user/${userId}`).then(r => r.json()),
});
```
### 5. Missing Cleanup for Subscriptions
**Problem:** Memory leaks, event handlers accumulate.
```typescript
// ❌ ANTI-PATTERN: No cleanup
useEffect(() => {
window.addEventListener('resize', handleResize);
}, []);
// ✅ CORRECT
useEffect(() => {
window.addEventListener('resize', handleResize);
return () => window.removeEventListener('resize', handleResize);
}, []);
```
## Dependency Array Anti-Patterns
### 1. Missing Dependencies (Stale Closures)
```typescript
// ❌ ANTI-PATTERN: count is stale
useEffect(() => {
const id = setInterval(() => {
setCount(count + increment); // Uses stale count!
}, 1000);
return () => clearInterval(id);
}, []); // Missing count and increment
// ✅ CORRECT: Functional update removes count dependency
useEffect(() => {
const id = setInterval(() => {
setCount(c => c + increment);
}, 1000);
return () => clearInterval(id);
}, [increment]);
```
### 2. Object/Array Dependencies (Infinite Loops)
```typescript
// ❌ ANTI-PATTERN: New object every render = infinite loop
const options = { userId, page: 1 };
useEffect(() => {
fetchData(options);
}, [options]); // options is new object every render!
// ✅ CORRECT: Use primitives
useEffect(() => {
fetchData({ userId, page: 1 });
}, [userId]);
// ✅ ALTERNATIVE: Memoize if object is needed
const options = useMemo(() => ({ userId, page: 1 }), [userId]);
```
### 3. eslint-disable for Dependencies
**This is almost always wrong.** The linter is catching real bugs.
```typescript
// ❌ ANTI-PATTERN: Hiding the bug
useEffect(() => {
doSomething(value);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []); // value will be stale!
// ✅ CORRECT: Fix the actual issue
useEffect(() => {
doSomething(value);
}, [value]);
// Or if you truly want "mount only", question why:
// - Is this initialization that should be in useState initializer?
// - Is this a subscription that should include cleanup?
```
## State Mutation Anti-Patterns
### Direct Array Mutation
```typescript
// ❌ ANTI-PATTERN: Same reference, no re-render
const addItem = (item: Item) => {
items.push(item);
setItems(items); // React sees same reference, skips update
};
// ✅ CORRECT
const addItem = (item: Item) => {
setItems([...items, item]);
};
```
### Direct Object Mutation
```typescript
// ❌ ANTI-PATTERN
const updateUser = (name: string) => {
user.name = name;
setUser(user); // Same reference
};
// ✅ CORRECT
const updateUser = (name: string) => {
setUser({ ...user, name });
};
```
### Immutable Operations Reference
| Operation | ❌ Mutating | ✅ Immutable |
|-----------|------------|-------------|
| Add | `arr.push(item)` | `[...arr, item]` |
| Add at start | `arr.unshift(item)` | `[item, ...arr]` |
| Remove | `arr.splice(i, 1)` | `arr.filter((_, idx) => idx !== i)` |
| Update | `arr[i] = newItem` | `arr.map((x, idx) => idx === i ? newItem : x)` |
| Sort | `arr.sort()` | `[...arr].sort()` |
| Reverse | `arr.reverse()` | `[...arr].reverse()` |
## Memoization Anti-Patterns
### 1. useMemo for Simple Calculations
```typescript
// ❌ ANTI-PATTERN: Overhead > benefit
const fullName = useMemo(
() => `${firstName} ${lastName}`,
[firstName, lastName]
);
// ✅ CORRECT: Just compute it
const fullName = `${firstName} ${lastName}`;
```
**Rule:** Only use `useMemo` for genuinely expensive calculations (>1ms) OR to maintain referential equality for `React.memo` children.
### 2. useCallback Without React.memo
```typescript
// ❌ ANTI-PATTERN: useCallback does nothing here
function Parent() {
const handleClick = useCallback(() => {
console.log('clicked');
}, []);
return <Child onClick={handleClick} />; // Child still re-renders!
}
// ✅ CORRECT: Combine with React.memo
const Child = React.memo(function Child({ onClick }: { onClick: () => void }) {
return <button onClick={onClick}>Click</button>;
});
function Parent() {
const handleClick = useCallback(() => {
console.log('clicked');
}, []);
return <Child onClick={handleClick} />; // Now Child skips re-render
}
```
### 3. Inline Objects/Functions in JSX to Memoized Children
```typescript
// ❌ ANTI-PATTERN: New object every render defeats memo
const MemoizedList = React.memo(List);
function Parent() {
return (
<MemoizedList
style={{ color: 'red' }} // New object every render!
onClick={() => handleClick()} // New function every render!
/>
);
}
// ✅ CORRECT: Stable references
const style = { color: 'red' }; // Or useMemo if dynamic
function Parent() {
const handleClick = useCallback(() => { /* ... */ }, []);
return <MemoizedList style={style} onClick={handleClick} />;
}
```
## TypeScript Anti-Patterns
### 1. Using `any` Type
```typescript
// ❌ ANTI-PATTERN
const data: any = await response.json();
console.log(data.user.name); // No type safety
// ✅ CORRECT
interface ApiResponse {
user: { name: string; email: string };
}
const data: ApiResponse = await response.json();
console.log(data.user.name); // Type safe
```
### 2. React.FC for Components
```typescript
// ❌ DISCOURAGED
const App: React.FC<AppProps> = ({ message }) => {
return <div>{message}</div>;
};
// ✅ PREFERRED
const App = ({ message }: AppProps) => {
return <div>{message}</div>;
};
```
**Why:** `React.FC` has issues with generics, defaultProps, and previously included implicit `children`.
### 3. Optional Props Creating Invalid States
```typescript
// ❌ ANTI-PATTERN: Can have value without onChange
interface InputProps {
value?: string;
onChange?: (value: string) => void;
}
// ✅ CORRECT: Discriminated union
type ControlledProps = { value: string; onChange: (v: string) => void };
type UncontrolledProps = { value?: never; onChange?: never; defaultValue?: string };
type InputProps = { label: string } & (ControlledProps | UncontrolledProps);
```
### 4. Array Access Without Undefined Check
```typescript
// ❌ ANTI-PATTERN (without noUncheckedIndexedAccess)
const arr: string[] = ['a', 'b'];
console.log(arr[10].toUpperCase()); // Runtime error!
// ✅ CORRECT
const item = arr[10];
if (item) {
console.log(item.toUpperCase());
}
```
Enable `noUncheckedIndexedAccess: true` in tsconfig.
## Component Anti-Patterns
### 1. Component Defined Inside Another Component
```typescript
// ❌ ANTI-PATTERN: Inner remounts every render
function Parent() {
function Child() { // New component identity each render!
return <div>Child</div>;
}
return <Child />;
}
// ✅ CORRECT: Define outside
function Child() {
return <div>Child</div>;
}
function Parent() {
return <Child />;
}
```
### 2. Index as Key in Dynamic Lists
```typescript
// ❌ ANTI-PATTERN: State corruption on reorder
{items.map((item, index) => (
<Item key={index} item={item} /> // BAD
))}
// ✅ CORRECT: Stable unique ID
{items.map(item => (
<Item key={item.id} item={item} />
))}
```
### 3. God Components (Too Much Responsibility)
**Symptoms:**
- Component > 300 lines
- 10+ pieces of state
- Multiple unrelated concerns
- Difficult to test in isolation
**Solution:** Split by concern, extract custom hooks.
### 4. Prop Drilling > 3 Levels
```typescript
// ❌ ANTI-PATTERN
<GrandParent user={user}>
<Parent user={user}>
<Child user={user}>
<GrandChild user={user} />
</Child>
</Parent>
</GrandParent>
// ✅ CORRECT: Context or composition
const UserContext = createContext<User | null>(null);
function GrandParent({ user }: { user: User }) {
return (
<UserContext value={user}>
<Parent />
</UserContext>
);
}
// Or composition pattern
function GrandParent({ user }: { user: User }) {
return (
<Parent>
<Child>
<GrandChild user={user} />
</Child>
</Parent>
);
}
```
## State Management Anti-Patterns
### 1. Copying Server State to Local State
```typescript
// ❌ ANTI-PATTERN: Creates stale data
const { data } = useQuery({ queryKey: ['todos'], queryFn: fetchTodos });
const [todos, setTodos] = useState<Todo[]>([]);
useEffect(() => {
if (data) setTodos(data);
}, [data]);
// ✅ CORRECT: Query is the source of truth
const { data: todos } = useQuery({ queryKey: ['todos'], queryFn: fetchTodos });
```
### 2. Global State for Local Concerns
```typescript
// ❌ ANTI-PATTERN: Modal state in global store
const useStore = create((set) => ({
isModalOpen: false,
toggleModal: () => set((s) => ({ isModalOpen: !s.isModalOpen })),
}));
// ✅ CORRECT: Local state
function FeatureWithModal() {
const [isOpen, setIsOpen] = useState(false);
return (
<>
<button onClick={() => setIsOpen(true)}>Open</button>
{isOpen && <Modal onClose={() => setIsOpen(false)} />}
</>
);
}
```
### 3. Zustand: Subscribing to Entire Store
```typescript
// ❌ ANTI-PATTERN: Re-renders on ANY store change
const { bears, fish } = useStore();
// ✅ CORRECT: Select only what you need
const bears = useStore((state) => state.bears);
const fish = useStore((state) => state.fish);
```
## File Structure Anti-Patterns
### Barrel Files in Application Code
```typescript
// ❌ ANTI-PATTERN: index.ts that re-exports everything
// src/components/index.ts
export * from './Button';
export * from './Input';
export * from './Modal';
// ... 50 more exports
// Importing one thing loads everything!
import { Button } from '@/components';
// ✅ CORRECT: Direct imports
import { Button } from '@/components/Button';
```
**Problems with barrel files:**
- Loads all exports even when importing one
- Circular dependency risks
- Tree-shaking breaks
- Slow dev server startup
**Exception:** Library public APIs where you control the entire export surface.references/checklist.md
# Code Review Checklist
## Critical Issues (Block Merge)
### React Hooks
- [ ] No `useEffect` that only computes derived state
- [ ] No missing cleanup functions (subscriptions, timers, fetch)
- [ ] No conditional hook calls (`if (x) { useState() }`)
- [ ] Dependency arrays are complete and correct
- [ ] No `eslint-disable react-hooks/exhaustive-deps` without strong justification
### State Management
- [ ] No direct state mutations (`.push()`, `.splice()`, `arr[i] = x`)
- [ ] No copying server data to local state (TanStack Query data → useState)
- [ ] State is colocated near where it's used
### React 19 Specific
- [ ] `useFormStatus` called in child component, not same component as `<form>`
- [ ] Promises passed to `use()` are not created inline in render
- [ ] Server Actions have `'use server'` directive
- [ ] `'use client'` boundary is as low as possible in component tree
### Keys
- [ ] No `index` as key in lists that can reorder, filter, or add/remove items
- [ ] Keys are stable and unique within siblings
### TypeScript
- [ ] No `any` type without explicit justification comment
- [ ] Array access handles possible `undefined` (or `noUncheckedIndexedAccess` enabled)
- [ ] Props interfaces use discriminated unions for mutually exclusive props
---
## High Priority Issues
### Component Structure
- [ ] No component defined inside another component (causes remount)
- [ ] Components under 300 lines (split if larger)
- [ ] Single responsibility - one reason to change
### Props and State
- [ ] Controlled inputs initialized with empty string, not `undefined`
- [ ] Props typed explicitly (not using `React.FC` with generics)
- [ ] No prop drilling beyond 2-3 levels
### Performance
- [ ] `useMemo`/`useCallback` justified by measurement or `React.memo` child
- [ ] No inline object/function in JSX passed to memoized children
- [ ] Large lists use virtualization (react-window, react-virtual)
### Error Handling
- [ ] Error boundaries at appropriate granularity
- [ ] Async operations have error handling
- [ ] Form validation shows clear error messages
---
## Architecture Checks
### Custom Hooks
- [ ] Hook names start with `use` followed by capital letter
- [ ] Hooks are reusable or significantly improve readability
- [ ] Non-hook logic extracted to utility functions instead
### State Management Strategy
- [ ] Server state uses TanStack Query (or similar)
- [ ] Client state uses appropriate solution:
- Local: `useState`/`useReducer`
- Global simple: Zustand
- Global derived: Jotai
- [ ] Form state uses React 19 `useActionState` where appropriate
### File Organization
- [ ] Feature-based folder structure for larger apps
- [ ] No barrel files (`index.ts`) in application code
- [ ] Shared components in `components/`
- [ ] Feature-specific code in `features/<name>/`
---
## TypeScript Quality
### Type Safety
- [ ] Strict mode enabled (`"strict": true`)
- [ ] `noUncheckedIndexedAccess: true` for array safety
- [ ] Generic components preserve type inference
- [ ] Event handlers properly typed (`React.MouseEvent<HTMLButtonElement>`)
### Type Patterns
- [ ] Discriminated unions for variant props
- [ ] `as const` for literal types where appropriate
- [ ] Explicit return types on exported functions
- [ ] No type assertions (`as`) without comment explaining why
### Context Typing
- [ ] Context created with `null` default has custom hook with null check
- [ ] Provider value is memoized if object
```typescript
// ✅ Correct context pattern
const MyContext = createContext<ContextType | null>(null);
function useMyContext() {
const ctx = useContext(MyContext);
if (!ctx) throw new Error('useMyContext must be within MyProvider');
return ctx;
}
function MyProvider({ children }: { children: ReactNode }) {
const value = useMemo(() => ({ /* ... */ }), [deps]);
return <MyContext value={value}>{children}</MyContext>;
}
```
---
## Performance Checks
### Re-render Prevention
- [ ] `React.memo` used for expensive pure components
- [ ] Callbacks passed to memoized children are stable (`useCallback`)
- [ ] Object props to memoized children are stable (`useMemo`)
- [ ] Zustand selectors select minimal state
### Bundle Size
- [ ] Route-level code splitting with `React.lazy`
- [ ] Heavy libraries imported dynamically where possible
- [ ] No unused dependencies
- [ ] Tree-shaking friendly imports (no `import * as`)
### Lists and Tables
- [ ] Virtualization for 100+ items
- [ ] Stable keys for all list items
- [ ] Filtered/sorted lists memoized if expensive
---
## Accessibility (a11y)
### Semantic HTML
- [ ] Interactive elements are `<button>`, `<a>`, `<input>`, etc. (not `<div onClick>`)
- [ ] Headings follow hierarchy (`h1` → `h2` → `h3`)
- [ ] Lists use `<ul>`/`<ol>`/`<li>`
### Forms
- [ ] All inputs have associated `<label>` (via `htmlFor` or wrapping)
- [ ] Required fields indicated visually and via `aria-required`
- [ ] Error messages associated via `aria-describedby`
### Keyboard Navigation
- [ ] All interactive elements focusable
- [ ] Visible focus indicators (don't remove `outline`)
- [ ] `tabIndex` used appropriately (0 or -1, rarely positive)
### Screen Readers
- [ ] Images have meaningful `alt` text (or `alt=""` if decorative)
- [ ] Icons have `aria-label` or `aria-hidden="true"`
- [ ] Dynamic content updates announced (`aria-live`)
### Links
- [ ] External links have `rel="noopener noreferrer"` with `target="_blank"`
- [ ] Link text is descriptive (not "click here")
---
## Testing Standards
### Query Priority
Use queries in this order (Testing Library best practice):
1. `getByRole` - accessible to everyone
2. `getByLabelText` - form fields
3. `getByPlaceholderText` - if no label
4. `getByText` - non-interactive elements
5. `getByTestId` - last resort
### Test Quality
- [ ] Tests verify behavior, not implementation
- [ ] `userEvent` used instead of `fireEvent`
- [ ] Async operations use `waitFor` or `findBy`
- [ ] Error states covered
- [ ] Edge cases covered (empty state, loading, error)
### Test Structure
- [ ] Arrange-Act-Assert pattern
- [ ] One assertion focus per test (can have multiple asserts)
- [ ] Test descriptions explain expected behavior
- [ ] No test interdependencies
---
## Quick Reference: Common Issues by Symptom
| Symptom | Likely Cause | Check |
|---------|--------------|-------|
| Infinite re-renders | Object in dependency array | New object created each render? |
| Stale state in callback | Missing dependency | Closure over old value? |
| State doesn't update | Direct mutation | Using `.push()` or direct assignment? |
| Controlled input warning | `undefined` initial value | Initialize with empty string |
| Hook error | Conditional hook call | Hook inside `if`/loop? |
| Memory leak warning | Missing cleanup | Subscription/timer without cleanup? |
| Type error on array access | Missing undefined check | Enable `noUncheckedIndexedAccess` |
| Child re-renders unnecessarily | Unstable props | Inline object/function to memo child? |
| Form status always false | Wrong component | `useFormStatus` in form component? |
---
## Review Comment Templates
### Critical Issue
```
🚫 **Critical:** [Issue description]
This will cause [bug/leak/crash] because [reason].
**Current:**
```code```
**Should be:**
```code```
```
### Suggestion
```
💡 **Suggestion:** [Improvement description]
This would improve [maintainability/performance/readability] by [reason].
**Consider:**
```code```
```
### Question
```
❓ **Question:** [Clarification needed]
Is this intentional? [Context for why you're asking]
```references/react19-patterns.md
# React 19 Patterns Reference
## useActionState (Forms)
Replaces manual `isLoading`, `error`, `data` state management for form actions.
```typescript
import { useActionState } from 'react';
interface FormState {
error: string | null;
success: boolean;
}
async function createPost(prevState: FormState, formData: FormData): Promise<FormState> {
const title = formData.get('title') as string;
if (!title) return { error: 'Title required', success: false };
await fetch('/api/posts', { method: 'POST', body: JSON.stringify({ title }) });
return { error: null, success: true };
}
function NewPostForm() {
const [state, formAction, isPending] = useActionState(createPost, {
error: null,
success: false,
});
return (
<form action={formAction}>
<input name="title" />
<button type="submit" disabled={isPending}>
{isPending ? 'Submitting...' : 'Submit'}
</button>
{state.error && <p className="error">{state.error}</p>}
</form>
);
}
```
**Review Points:**
- Action function must return consistent object shape
- Actions should be pure (return new state, not mutate)
- Check that error handling covers all failure cases
## useFormStatus (Submit Button State)
**Critical Rule:** Must be called in a child component of `<form>`, NOT in the same component.
```typescript
import { useFormStatus } from 'react-dom';
// ✅ CORRECT: Child component
function SubmitButton({ children }: { children: React.ReactNode }) {
const { pending, data, method, action } = useFormStatus();
return (
<button type="submit" disabled={pending}>
{pending ? 'Submitting...' : children}
</button>
);
}
function MyForm() {
return (
<form action={submitAction}>
<input name="email" />
<SubmitButton>Send</SubmitButton>
</form>
);
}
// ❌ WRONG: Same component as form - pending is always false!
function BadForm() {
const { pending } = useFormStatus(); // BUG: Always false
return (
<form action={submitAction}>
<button disabled={pending}>Submit</button>
</form>
);
}
```
## useOptimistic (Instant UI Updates)
Provides instant feedback before server confirms. Automatically reverts on error.
```typescript
import { useOptimistic, startTransition } from 'react';
interface Message {
id: string;
text: string;
sending?: boolean;
}
function MessageThread({ messages }: { messages: Message[] }) {
const [optimisticMessages, addOptimisticMessage] = useOptimistic(
messages,
(currentMessages, newText: string) => [
{ id: crypto.randomUUID(), text: newText, sending: true },
...currentMessages,
]
);
async function sendMessage(formData: FormData) {
const text = formData.get('message') as string;
addOptimisticMessage(text);
startTransition(async () => {
await deliverMessage(text);
});
}
return (
<div>
{optimisticMessages.map(msg => (
<div key={msg.id} style={{ opacity: msg.sending ? 0.5 : 1 }}>
{msg.text}
</div>
))}
<form action={sendMessage}>
<input name="message" />
<button type="submit">Send</button>
</form>
</div>
);
}
```
**Review Points:**
- Optimistic state should be visually distinct (opacity, spinner)
- Consider what happens on error (automatic revert)
- Wrap async operations in `startTransition`
## use() API (Promise and Context Reading)
Unlike hooks, `use()` can be called conditionally. Major departure from traditional React rules.
### Reading Promises
```typescript
import { use, Suspense } from 'react';
// ✅ CORRECT: Promise passed from parent
function Comments({ commentsPromise }: { commentsPromise: Promise<Comment[]> }) {
const comments = use(commentsPromise); // Suspends until resolved
return <ul>{comments.map(c => <li key={c.id}>{c.text}</li>)}</ul>;
}
function Page() {
const commentsPromise = fetchComments(); // Created once
return (
<Suspense fallback={<Spinner />}>
<Comments commentsPromise={commentsPromise} />
</Suspense>
);
}
// ❌ WRONG: Promise created in render - INFINITE LOOP
function BadComponent() {
const data = use(fetch('/api').then(r => r.json())); // New promise every render!
}
```
### Conditional Context Reading
```typescript
import { use, createContext } from 'react';
const ThemeContext = createContext<string>('light');
// ✅ use() can be called conditionally (useContext cannot!)
function HorizontalRule({ show }: { show: boolean }) {
if (show) {
const theme = use(ThemeContext);
return <hr className={`hr-${theme}`} />;
}
return null;
}
```
**Review Points:**
- Always wrap `use(promise)` in `<Suspense>`
- Promise must come from props, state, or outside component
- Never create promise inline in render
## Server vs Client Components
| Server Components | Client Components |
|-------------------|-------------------|
| No directive needed | Must start with `'use client'` |
| Can access DB, filesystem | Can use useState, useEffect |
| Zero client JS | Ships JS to browser |
| Cannot use hooks | Full interactivity |
### Boundary Placement
Push `'use client'` as low as possible in the tree.
```typescript
// ✅ GOOD: Only interactive part is Client Component
// ProductPage.tsx (Server Component - no directive)
async function ProductPage({ id }: { id: string }) {
const product = await db.query(`SELECT * FROM products WHERE id = $1`, [id]);
return (
<article>
<h1>{product.name}</h1> {/* Server: no JS */}
<p>{product.description}</p> {/* Server: no JS */}
<AddToCartButton productId={id} /> {/* Client: needs onClick */}
</article>
);
}
// AddToCartButton.tsx
'use client';
function AddToCartButton({ productId }: { productId: string }) {
const [adding, setAdding] = useState(false);
// Client-side interactivity
}
// ❌ BAD: Entire page is client-side unnecessarily
'use client';
function ProductPage({ id }) {
// Everything ships to client even though most is static
}
```
### Serialization Constraint
Props passed from Server to Client Components must be serializable.
**Cannot pass:**
- Functions (except Server Actions)
- Classes
- Symbols
- DOM nodes
```typescript
// ❌ WRONG: Function prop from Server to Client
async function ServerParent() {
const handleClick = () => console.log('clicked'); // Not serializable!
return <ClientChild onClick={handleClick} />;
}
// ✅ CORRECT: Use Server Action
async function ServerParent() {
async function handleSubmit(formData: FormData) {
'use server';
// Server-side logic
}
return <ClientChild onSubmit={handleSubmit} />;
}
```
## ref as Prop (React 19)
No more `forwardRef` needed - ref can be passed as a regular prop.
```typescript
// React 18: Required forwardRef
const Input = forwardRef<HTMLInputElement, InputProps>((props, ref) => (
<input ref={ref} {...props} />
));
// React 19: ref is just a prop
function Input({ ref, ...props }: InputProps & { ref?: React.Ref<HTMLInputElement> }) {
return <input ref={ref} {...props} />;
}
```
## Context as Provider
`<Context>` can be used directly as provider (no `.Provider` needed).
```typescript
// React 18
<ThemeContext.Provider value={theme}>
{children}
</ThemeContext.Provider>
// React 19
<ThemeContext value={theme}>
{children}
</ThemeContext>
```
## Document Metadata in Components
Title and meta tags can be rendered anywhere in the tree.
```typescript
function BlogPost({ post }: { post: Post }) {
return (
<article>
<title>{post.title}</title>
<meta name="description" content={post.excerpt} />
<h1>{post.title}</h1>
<p>{post.content}</p>
</article>
);
}
```
React 19 automatically hoists these to `<head>`.SKILL.md
---
name: typescript-react-reviewer
description: "Expert code reviewer for TypeScript + React 19 applications. Use when reviewing React code, identifying anti-patterns, evaluating state management, or assessing code maintainability. Triggers: code review requests, PR reviews, React architecture evaluation, identifying code smells, TypeScript type safety checks, useEffect abuse detection, state management review."
---
# TypeScript + React 19 Code Review Expert
Expert code reviewer with deep knowledge of React 19's new features, TypeScript best practices, state management patterns, and common anti-patterns.
## Review Priority Levels
### 🚫 Critical (Block Merge)
These issues cause bugs, memory leaks, or architectural problems:
| Issue | Why It's Critical |
|-------|-------------------|
| `useEffect` for derived state | Extra render cycle, sync bugs |
| Missing cleanup in `useEffect` | Memory leaks |
| Direct state mutation (`.push()`, `.splice()`) | Silent update failures |
| Conditional hook calls | Breaks Rules of Hooks |
| `key={index}` in dynamic lists | State corruption on reorder |
| `any` type without justification | Type safety bypass |
| `useFormStatus` in same component as `<form>` | Always returns false (React 19 bug) |
| Promise created inside render with `use()` | Infinite loop |
### ⚠️ High Priority
| Issue | Impact |
|-------|--------|
| Incomplete dependency arrays | Stale closures, missing updates |
| Props typed as `any` | Runtime errors |
| Unjustified `useMemo`/`useCallback` | Unnecessary complexity |
| Missing Error Boundaries | Poor error UX |
| Controlled input initialized with `undefined` | React warning |
### 📝 Architecture/Style
| Issue | Recommendation |
|-------|----------------|
| Component > 300 lines | Split into smaller components |
| Prop drilling > 2-3 levels | Use composition or context |
| State far from usage | Colocate state |
| Custom hooks without `use` prefix | Follow naming convention |
## Quick Detection Patterns
### useEffect Abuse (Most Common Anti-Pattern)
```typescript
// ❌ WRONG: Derived state in useEffect
const [firstName, setFirstName] = useState('');
const [fullName, setFullName] = useState('');
useEffect(() => {
setFullName(firstName + ' ' + lastName);
}, [firstName, lastName]);
// ✅ CORRECT: Compute during render
const fullName = firstName + ' ' + lastName;
```
```typescript
// ❌ WRONG: Event logic in useEffect
useEffect(() => {
if (product.isInCart) showNotification('Added!');
}, [product]);
// ✅ CORRECT: Logic in event handler
function handleAddToCart() {
addToCart(product);
showNotification('Added!');
}
```
### React 19 Hook Mistakes
```typescript
// ❌ WRONG: useFormStatus in form component (always returns false)
function Form() {
const { pending } = useFormStatus();
return <form action={submit}><button disabled={pending}>Send</button></form>;
}
// ✅ CORRECT: useFormStatus in child component
function SubmitButton() {
const { pending } = useFormStatus();
return <button type="submit" disabled={pending}>Send</button>;
}
function Form() {
return <form action={submit}><SubmitButton /></form>;
}
```
```typescript
// ❌ WRONG: Promise created in render (infinite loop)
function Component() {
const data = use(fetch('/api/data')); // New promise every render!
}
// ✅ CORRECT: Promise from props or state
function Component({ dataPromise }: { dataPromise: Promise<Data> }) {
const data = use(dataPromise);
}
```
### State Mutation Detection
```typescript
// ❌ WRONG: Mutations (no re-render)
items.push(newItem);
setItems(items);
arr[i] = newValue;
setArr(arr);
// ✅ CORRECT: Immutable updates
setItems([...items, newItem]);
setArr(arr.map((x, idx) => idx === i ? newValue : x));
```
### TypeScript Red Flags
```typescript
// ❌ Red flags to catch
const data: any = response; // Unsafe any
const items = arr[10]; // Missing undefined check
const App: React.FC<Props> = () => {}; // Discouraged pattern
// ✅ Preferred patterns
const data: ResponseType = response;
const items = arr[10]; // with noUncheckedIndexedAccess
const App = ({ prop }: Props) => {}; // Explicit props
```
## Review Workflow
1. **Scan for critical issues first** - Check for the patterns in "Critical (Block Merge)" section
2. **Check React 19 usage** - See [react19-patterns.md](references/react19-patterns.md) for new API patterns
3. **Evaluate state management** - Is state colocated? Server state vs client state separation?
4. **Assess TypeScript safety** - Generic components, discriminated unions, strict config
5. **Review for maintainability** - Component size, hook design, folder structure
## Reference Documents
For detailed patterns and examples:
- **[react19-patterns.md](references/react19-patterns.md)** - React 19 new hooks (useActionState, useOptimistic, use), Server/Client Component boundaries
- **[antipatterns.md](references/antipatterns.md)** - Comprehensive anti-pattern catalog with fixes
- **[checklist.md](references/checklist.md)** - Full code review checklist for thorough reviews
## State Management Quick Guide
| Data Type | Solution |
|-----------|----------|
| Server/async data | TanStack Query (never copy to local state) |
| Simple global UI state | Zustand (~1KB, no Provider) |
| Fine-grained derived state | Jotai (~2.4KB) |
| Component-local state | useState/useReducer |
| Form state | React 19 useActionState |
### TanStack Query Anti-Pattern
```typescript
// ❌ NEVER copy server data to local state
const { data } = useQuery({ queryKey: ['todos'], queryFn: fetchTodos });
const [todos, setTodos] = useState([]);
useEffect(() => setTodos(data), [data]);
// ✅ Query IS the source of truth
const { data: todos } = useQuery({ queryKey: ['todos'], queryFn: fetchTodos });
```
## TypeScript Config Recommendations
```json
{
"compilerOptions": {
"strict": true,
"noUncheckedIndexedAccess": true,
"noImplicitReturns": true,
"exactOptionalPropertyTypes": true
}
}
```
`noUncheckedIndexedAccess` is critical - it catches `arr[i]` returning undefined.
## Immediate Red Flags
When reviewing, flag these immediately:
| Pattern | Problem | Fix |
|---------|---------|-----|
| `eslint-disable react-hooks/exhaustive-deps` | Hides stale closure bugs | Refactor logic |
| Component defined inside component | Remounts every render | Move outside |
| `useState(undefined)` for inputs | Uncontrolled warning | Use empty string |
| `React.FC` with generics | Generic inference breaks | Use explicit props |
| Barrel files (`index.ts`) in app code | Bundle bloat, circular deps | Direct imports |