DEV Community: Tarun Moorjani

Self-Review: Catching Your Own Mistakes Before Others Do

Tarun Moorjani — Mon, 09 Mar 2026 18:04:02 +0000

I used to submit PRs the moment my code worked. Hit "Create Pull Request" and wait for feedback.

The result:

First review: "You left console.log statements"
Second review: "Tests are failing"
Third review: "This variable name is misleading"
Fourth review: "You forgot to update the documentation"
Fifth review: Finally approved

Five review cycles. Three days. Multiple reviewers annoyed.

Then I learned to self-review. Now:

One review cycle
Same day approval
Reviewers actually thank me for thorough PRs

The secret: Spend 10 minutes reviewing your own code before anyone else sees it.

Here's exactly how I do it.

Why Self-Review Matters

The Embarrassment Factor

Nothing feels worse than:

Reviewer pointing out you left console.log('DEBUG: user data:', user)
Finding out tests are failing (didn't run them locally)
Realizing you committed commented-out code
Someone asking "What does temp2 mean?"

Self-review eliminates 80% of embarrassing feedback.

The Time Factor

Without self-review:

Submit PR → Wait 4 hours → Review feedback → Fix issues → 
Wait 4 hours → More feedback → Fix more → Wait 4 hours → Approved

Total: 2-3 days, 3+ review cycles

With self-review:

Self-review 10 minutes → Submit clean PR → Wait 4 hours → Approved

Total: 4 hours, 1 review cycle

Time saved: 2+ days per PR

The Relationship Factor

Reviewers notice when you consistently submit clean PRs. They:

Review your PRs faster
Give more thoughtful feedback
Trust your code more
Recommend you for senior positions

Self-review is a career accelerator.

The 10-Minute Self-Review Process

Here's my exact process. Takes 10 minutes. Saves hours.

Step 1: Wait 5 Minutes (Seriously)

Before reviewing, take a break:

Get coffee
Walk around
Check messages
Stretch

Why: Fresh eyes catch things you miss when you're "in the zone."

Your brain was in "make it work" mode. You need "does this make sense?" mode.

Step 2: Review on GitHub/GitLab (Not Your IDE)

Don't review in VS Code. Review in GitHub.

Why:

✅ Same view reviewers will see
✅ Catches formatting issues
✅ Forces you to read like a reviewer
❌ IDE familiarity makes you blind to issues

Action:

Create draft PR
Click "Files changed"
Review every file as if you're the reviewer

Step 3: Read Your Own Code Out Loud

Literally read your changes out loud (or silently mouth the words).

// Reading this out loud:
"Function process user takes user... 
returns user dot data dot profile dot email...
Wait, what if profile is null?"

Catches: Logic errors, missing null checks, confusing code

Step 4: Run the Checklist (See Below)

Go through the complete checklist systematically.

Don't skip items. Each one catches real issues.

Step 5: Use AI to Review Your Code

Game changer: Let AI catch what you miss.

# Use Claude Code
git diff main | claude code --skill self-review-check

# Or paste into Claude/ChatGPT
"Review this diff for issues I might have missed:
- Console.logs
- TODO comments
- Commented code
- Poor variable names
- Missing tests
- Type safety issues
[paste diff]"

AI catches:

Things you're blind to (console.logs you added while debugging)
Patterns you don't notice (same logic in multiple files)
Edge cases you didn't test
Documentation gaps

Step 6: Test One More Time

Even if tests passed before:

# Clean install and test
rm -rf node_modules
npm install
npm test
npm run build
npm run lint

Why: Catches:

Tests that only pass with cached dependencies
Linting errors you ignored
Build failures in production mode
Type errors you suppressed locally

The Complete Self-Review Checklist

Copy this. Use it every time.

Code Quality Check

Basic Cleanup:

[ ] No console.log statements (use logger.debug instead)
[ ] No commented-out code (delete it, it's in git history)
[ ] No TODO comments (create GitHub issues instead)
[ ] No temporary test data (hardcoded values)
[ ] No "temp" or "test" variable names
[ ] No debug code left in

Code Style:

[ ] Consistent naming conventions (camelCase, PascalCase)
[ ] No single-letter variables (except loop counters)
[ ] Functions are small and focused (< 50 lines)
[ ] No deeply nested logic (> 3 levels)
[ ] Clear function and variable names
[ ] Code is formatted (Prettier/ESLint)

Example - Before self-review:

// ❌ This would embarrass you in review
function processStuff(data: any) {
  console.log('DEBUG:', data); // Left in
  const temp = data.users; // Vague name
  // const old_code = temp.filter(x => x.active); // Commented out
  // TODO: optimize this later

  for(let i=0; i<temp.length; i++) {
    for(let j=0; j<temp[i].posts.length; j++) {
      for(let k=0; k<temp[i].posts[j].comments.length; k++) {
        // Nested 3 levels deep
        if(temp[i].posts[j].comments[k].flagged) {
          console.log('found one!');
        }
      }
    }
  }
}

After self-review:

// ✅ Clean, professional
function getFlaggedComments(users: User[]): Comment[] {
  return users.flatMap(user =>
    user.posts.flatMap(post =>
      post.comments.filter(comment => comment.flagged)
    )
  );
}

TypeScript Safety Check

Type Safety:

[ ] No any types (or justified with comment)
[ ] No @ts-ignore (or explained why needed)
[ ] No @ts-expect-error without TODO issue
[ ] Type assertions are justified
[ ] All function parameters are typed
[ ] Return types are explicit for public APIs
[ ] Null/undefined handled properly
[ ] No implicit any from missing types

Type-Specific Issues:

[ ] Optional chaining used (?.) where needed
[ ] Nullish coalescing used (??) instead of ||
[ ] Type imports use import type syntax
[ ] Discriminated unions for state machines
[ ] Generic constraints are correct
[ ] No unsafe type assertions

Example - Common type issues:

// ❌ Would get flagged in review
function getUser(id: string) { // Missing return type
  const user = users.find(u => u.id === id)!; // Unsafe !
  return user.profile.email; // What if profile is null?
}

const data: any = await response.json(); // any!

// ✅ After self-review
function getUser(id: string): string | null {
  const user = users.find(u => u.id === id);
  return user?.profile?.email ?? null;
}

const UserSchema = z.object({ /* ... */ });
const data = UserSchema.parse(await response.json());

React Patterns Check

Hooks:

[ ] Hooks follow rules (no conditional hooks)
[ ] useEffect dependencies are correct
[ ] No missing dependencies (ESLint warning)
[ ] Cleanup functions for subscriptions
[ ] No infinite loops (missing deps causing re-runs)

Performance:

[ ] Expensive calculations use useMemo
[ ] Callback functions use useCallback
[ ] Heavy components use React.memo
[ ] No unnecessary object/array creation in render
[ ] Large lists are virtualized

State Management:

[ ] State is not derived from props (unless intentional)
[ ] No direct state mutation
[ ] State updates use functional form when needed
[ ] Complex state uses useReducer

Example - Common React issues:

// ❌ Would get flagged
function Component({ userId }) {
  const [user, setUser] = useState(null);

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

  const config = { theme: 'dark' }; // New object every render

  return <Child config={config} />; // Child re-renders unnecessarily
}

// ✅ After self-review
function Component({ userId }) {
  const [user, setUser] = useState(null);

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

  const config = useMemo(() => ({ theme: 'dark' }), []); // Memoized

  return <Child config={config} />;
}

Testing Check

Test Coverage:

[ ] Unit tests for new functions
[ ] Integration tests for new features
[ ] Tests actually test something (not just smoke tests)
[ ] Edge cases are tested
[ ] Error cases are tested
[ ] Tests have descriptive names

Test Quality:

[ ] No commented-out tests
[ ] No .only or .skip left in
[ ] Tests are independent (no shared state)
[ ] Mock data is realistic
[ ] Assertions are specific (not just "exists")

Test Execution:

[ ] All tests pass locally
[ ] Tests pass consistently (not flaky)
[ ] Coverage didn't decrease
[ ] Tests run in reasonable time

Example - Test issues to catch:

// ❌ Would get flagged
describe.only('UserService', () => { // Left .only in!
  it('works', () => { // Vague description
    const result = getUser('1');
    expect(result).toBeTruthy(); // Weak assertion
  });

  // it('handles errors', () => {}); // Commented out test
});

// ✅ After self-review
describe('UserService', () => {
  it('returns user when ID exists', () => {
    const result = getUser('1');
    expect(result).toEqual({
      id: '1',
      name: 'Test User',
      email: 'test@example.com'
    });
  });

  it('returns null when ID does not exist', () => {
    const result = getUser('nonexistent');
    expect(result).toBeNull();
  });

  it('throws error when ID is invalid format', () => {
    expect(() => getUser('')).toThrow('Invalid user ID');
  });
});

Performance Check

Bundle Size:

[ ] No large library imports (import only what you need)
[ ] Heavy components are code-split
[ ] Images are optimized
[ ] No duplicate dependencies

Runtime Performance:

[ ] No expensive operations in render
[ ] API calls are debounced/throttled
[ ] Large lists use virtualization
[ ] No memory leaks (listeners cleaned up)

Network:

[ ] API calls are batched where possible
[ ] Requests can be cancelled
[ ] Data is cached appropriately
[ ] No waterfall requests (parallel when possible)

Example - Performance issues:

// ❌ Would slow down app
import _ from 'lodash'; // Imports entire lodash!

function Component({ items }) {
  const sorted = items
    .filter(i => i.active)
    .sort((a, b) => b.date - a.date); // Runs every render!

  return <List items={sorted} />;
}

// ✅ After self-review
import sortBy from 'lodash/sortBy'; // Import only what's needed

function Component({ items }) {
  const sorted = useMemo(
    () => items
      .filter(i => i.active)
      .sort((a, b) => b.date - a.date),
    [items] // Only re-sort when items change
  );

  return <List items={sorted} />;
}

Security Check

Sensitive Data:

[ ] No API keys in code (use env variables)
[ ] No passwords or secrets hardcoded
[ ] No sensitive data in logs
[ ] No sensitive data in error messages

Input Validation:

[ ] User input is validated
[ ] User input is sanitized
[ ] No SQL injection risk
[ ] No XSS vulnerabilities

Authentication/Authorization:

[ ] Auth checks are in place
[ ] Tokens are stored securely
[ ] Sessions are handled properly
[ ] CSRF protection exists

Example - Security issues:

// ❌ Security issues
const API_KEY = 'sk-1234567890'; // Hardcoded!

function searchUsers(query: string) {
  // SQL injection risk!
  const sql = `SELECT * FROM users WHERE name = '${query}'`;
  return db.query(sql);
}

function renderComment(comment: string) {
  // XSS vulnerability!
  return <div dangerouslySetInnerHTML={{ __html: comment }} />;
}

console.log('User password:', password); // Logging sensitive data!

// ✅ After self-review
const API_KEY = process.env.REACT_APP_API_KEY; // From env

function searchUsers(query: string) {
  // Parameterized query
  return db.query('SELECT * FROM users WHERE name = ?', [query]);
}

function renderComment(comment: string) {
  // React escapes by default
  return <div>{comment}</div>;
}

logger.info('User logged in', { userId: user.id }); // No sensitive data

Accessibility Check

Semantic HTML:

[ ] Using semantic elements (, , )
[ ] Proper heading hierarchy
[ ] Lists use
[ ] Forms use element

ARIA:

[ ] ARIA labels on icon-only buttons
[ ] ARIA live regions for dynamic content
[ ] ARIA states (expanded, selected, etc.)
[ ] No redundant ARIA

Keyboard Navigation:

[ ] All interactive elements focusable
[ ] Tab order is logical
[ ] Focus visible on all elements
[ ] Keyboard shortcuts don't conflict

Screen Readers:

[ ] Images have alt text
[ ] Form labels are associated
[ ] Error messages are announced
[ ] Loading states are announced

Example - Accessibility issues:

// ❌ Accessibility issues
<div onClick={handleClick}>Click me</div> // Not keyboard accessible

<button onClick={handleClose}>
  <X /> {/* Icon only, no label */}
</button>

<img src="profile.jpg" /> {/* No alt text */}

<input type="text" /> {/* No label */}

// ✅ After self-review
<button onClick={handleClick}>Click me</button>

<button onClick={handleClose} aria-label="Close dialog">
  <X />
</button>

<img src="profile.jpg" alt="User profile picture" />

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

Documentation Check

Code Comments:

[ ] Complex logic is explained
[ ] Non-obvious decisions are documented
[ ] Workarounds have explanations
[ ] Public APIs are documented

README Updates:

[ ] New features documented
[ ] Breaking changes noted
[ ] Examples updated
[ ] Installation steps current

API Documentation:

[ ] New endpoints documented
[ ] Request/response examples provided
[ ] Error codes documented
[ ] Rate limits noted

Example - Documentation issues:

// ❌ No context for future developers
function calculateScore(a, b, c) {
  return (a * 0.4) + (b * 0.35) + (c * 0.25); // Magic numbers!
}

// ✅ After self-review
/**
 * Calculates user engagement score based on weighted metrics.
 * 
 * @param posts - Number of posts (40% weight)
 * @param comments - Number of comments (35% weight)
 * @param reactions - Number of reactions received (25% weight)
 * @returns Score from 0-100
 * 
 * Weights determined by engagement analysis in Q3 2024.
 * See: https://docs.company.com/engagement-scoring
 */
function calculateEngagementScore(
  posts: number,
  comments: number,
  reactions: number
): number {
  const POST_WEIGHT = 0.4;
  const COMMENT_WEIGHT = 0.35;
  const REACTION_WEIGHT = 0.25;

  return (posts * POST_WEIGHT) + 
         (comments * COMMENT_WEIGHT) + 
         (reactions * REACTION_WEIGHT);
}

Git Hygiene Check

Commits:

[ ] Commit messages are clear
[ ] No "WIP" or "temp" commits
[ ] Commits are logical chunks
[ ] No merge commits (rebased)

Branch:

[ ] Branch is up to date with main
[ ] No merge conflicts
[ ] Branch name is descriptive

Files:

[ ] No unnecessary files committed
[ ] No .env files or secrets
[ ] No large binary files (unless necessary)
[ ] .gitignore is updated

Example - Git issues:

# ❌ Bad commit history
git log
> WIP
> more stuff
> fixes
> actually works now
> Merge branch 'main'
> temp

# ✅ After self-review cleanup
git rebase -i main
# Squash into meaningful commits:
> Add user profile validation
> Fix null pointer in profile fetch
> Update tests for new validation

Common Things Developers Miss

1. Console.log Statements

Everyone does this. You add debugging, code works, you forget to remove it.

Self-review catch:

# Search before submitting
git diff main | grep -i console

# Or use pre-commit hook

2. Commented-Out Code

"I'll uncomment it later" - No you won't. Delete it.

// ❌ Looks unprofessional
function process() {
  // const oldWay = data.map(x => x.value);
  // const anotherOldWay = data.filter(x => x.active);
  const newWay = data.filter(x => x.active).map(x => x.value);
  return newWay;
}

// ✅ Clean
function process() {
  return data.filter(x => x.active).map(x => x.value);
}

It's in git history if you need it.

3. TODO Comments

Create a GitHub issue instead.

// ❌ Will never get done
function uploadFile() {
  // TODO: add validation
  // TODO: handle large files
  // TODO: add progress indicator
  upload(file);
}

// ✅ Track properly
function uploadFile() {
  // FIXME: Add file validation (Issue #1234)
  // FIXME: Handle large files >100MB (Issue #1235)
  upload(file);
}

4. Poor Variable Names

You know what temp2 means now. Will you in 6 months?

// ❌ Future you will hate this
const temp = data.filter(x => x.active);
const temp2 = temp.map(x => x.value);
const result = temp2.sort();

// ✅ Self-documenting
const activeItems = data.filter(item => item.active);
const itemValues = activeItems.map(item => item.value);
const sortedValues = itemValues.sort();

// ✅ Even better - one clear chain
const sortedActiveValues = data
  .filter(item => item.active)
  .map(item => item.value)
  .sort();

5. Missing Null Checks

"It should never be null" - Famous last words.

// ❌ Will crash eventually
function getUserEmail(userId: string): string {
  const user = users.find(u => u.id === userId);
  return user.profile.email; // What if user not found?
}

// ✅ Defensive
function getUserEmail(userId: string): string | null {
  const user = users.find(u => u.id === userId);
  if (!user) return null;

  return user.profile?.email ?? null;
}

6. Unhandled Promises

Silent failures are the worst bugs.

// ❌ Error disappears
useEffect(() => {
  fetchData(); // Promise rejection unhandled!
}, []);

// ✅ Proper error handling
useEffect(() => {
  fetchData().catch(error => {
    console.error('Failed to fetch:', error);
    setError(error);
  });
}, []);

7. Overly Complex Logic

If you need to re-read your own code, it's too complex.

// ❌ What does this even do?
const x = a.map(b => b.c).filter((d, e) => 
  d && d.f && d.f.g ? d.f.g.h === i[e].j : false
).reduce((k, l) => k ? {...k, [l.m]: l.n} : {}, {});

// ✅ Broken into steps
const userProfiles = users.map(user => user.profile);
const validProfiles = userProfiles.filter(
  (profile, index) => {
    if (!profile?.settings?.preferences) return false;
    return profile.settings.preferences.theme === themes[index].default;
  }
);
const preferencesMap = validProfiles.reduce(
  (map, profile) => ({
    ...map,
    [profile.id]: profile.preferences
  }),
  {}
);

8. Copy-Paste Without Updating

Classic mistake: Copy function, forget to update names/comments.

// ❌ Copied and pasted, didn't update
/**
 * Gets user by ID
 */
function getPost(id: string): Post {
  return posts.find(p => p.id === id); // Wrong comment!
}

// ✅ Updated properly
/**
 * Gets post by ID
 */
function getPost(id: string): Post | undefined {
  return posts.find(post => post.id === id);
}

9. Hardcoded Test Data

Remove your test values before committing.

// ❌ Test data in production code
function Component() {
  const [email, setEmail] = useState('test@test.com');
  const [password, setPassword] = useState('password123');
  // ...
}

// ✅ Clean
function Component() {
  const [email, setEmail] = useState('');
  const [password, setPassword] = useState('');
  // ...
}

10. Incomplete Cleanup

If you tried something and reverted, clean up completely.

// ❌ Leftovers from experimentation
import { OldComponent } from './old'; // Not used anymore
import { NewComponent } from './new';

interface OldProps { /* ... */ } // Not used

function Component() {
  // const [oldState, setOldState] = useState(); // Commented out
  const [newState, setNewState] = useState();

  return <NewComponent state={newState} />;
}

// ✅ Clean
import { NewComponent } from './new';

function Component() {
  const [state, setState] = useState();
  return <NewComponent state={state} />;
}

Tools for Self-Review

1. Claude Code Skills for Self-Review

Create .claude/skills/self-review.md:

# Self-Review Checker

You are reviewing code before it gets submitted for PR review.

## Your Task

Analyze this git diff and flag issues the developer likely missed.

## Check For

**Obvious Mistakes:**
- console.log statements
- console.error, console.warn, console.debug
- debugger statements
- Commented-out code
- TODO/FIXME comments without issue numbers
- "temp", "test", "debug" variable names
- WIP or test code

**Type Safety:**
- Explicit `any` types
- `@ts-ignore` or `@ts-expect-error`
- Unsafe non-null assertions (!)
- Missing null checks
- Type assertions without validation

**Code Quality:**
- Functions longer than 50 lines
- Nested logic deeper than 3 levels
- Duplicate code
- Magic numbers without constants
- Poor variable names

**React-Specific:**
- Missing useEffect dependencies
- Expensive operations not memoized
- Unnecessary re-renders
- Missing cleanup functions

**Performance:**
- Large library imports
- Unoptimized images
- Synchronous expensive operations

**Security:**
- Hardcoded secrets or API keys
- Sensitive data in logs
- Unvalidated user input
- Potential XSS

**Testing:**
- `.only` or `.skip` in tests
- Commented-out tests
- Tests without assertions

**Git Hygiene:**
- Large binary files
- Committed .env files
- Uncommitted package-lock.json changes

## Output Format

### 🔴 Critical Issues (Fix before submitting)
[List with file:line and explanation]

### 🟡 Warnings (Should fix)
[List with file:line and explanation]

### 🟢 Suggestions (Nice to have)
[List with file:line and explanation]

### ✅ Looks Good
[What looks good in the PR]

## Be Specific

Include file names and line numbers.
Explain WHY it's an issue.
Suggest a fix.

Usage:

# Before submitting PR
git diff main | claude code --skill self-review

Result: AI catches what you're blind to.

2. GitHub Copilot for Quick Checks

In VS Code:

@workspace Review my staged changes for common issues:
- Debugging code left in
- Type safety issues
- Missing error handling
- Poor naming

Copilot will scan and flag issues.

3. ESLint + TypeScript

Essential setup:

npm install --save-dev \
  eslint \
  @typescript-eslint/eslint-plugin \
  @typescript-eslint/parser

Run before submitting:

npm run lint

Pre-commit hook:

{
  "husky": {
    "hooks": {
      "pre-commit": "npm run lint && npm test"
    }
  }
}

4. Pretty Quick

Format only changed files:

npm install --save-dev pretty-quick

# Add to package.json
{
  "scripts": {
    "format": "pretty-quick --staged"
  }
}

Pre-commit hook:

{
  "husky": {
    "hooks": {
      "pre-commit": "pretty-quick --staged && npm test"
    }
  }
}

5. Danger.js

Automated self-review checks:

// dangerfile.js
import { danger, warn, fail, message } from 'danger';

// Check for console.log
const consoleStatements = danger.git.modified_files
  .filter(file => file.endsWith('.ts') || file.endsWith('.tsx'))
  .map(file => {
    const content = danger.git.fileMatch(file);
    // Check for console statements
  });

if (consoleStatements.length > 0) {
  fail('console.log found. Remove debugging code.');
}

// Check for large PRs
const bigPR = danger.github.pr.additions + danger.github.pr.deletions > 500;
if (bigPR) {
  warn('This PR is large. Consider splitting it.');
}

// Check for missing tests
const hasAppChanges = danger.git.modified_files.some(
  f => f.match(/src\//) && f.endsWith('.tsx')
);
const hasTestChanges = danger.git.modified_files.some(
  f => f.includes('test')
);

if (hasAppChanges && !hasTestChanges) {
  warn('No tests added. Consider adding tests.');
}

// Require PR description
if (danger.github.pr.body.length < 50) {
  fail('Please add a proper PR description.');
}

// Check for TODOs
const newTodos = danger.git.modified_files.filter(file => {
  const content = danger.git.fileMatch(file);
  // Check for TODO without issue number
});

if (newTodos.length > 0) {
  warn('TODO comments found. Create GitHub issues instead.');
}

6. Git Hooks with Husky

Comprehensive pre-commit checks:

npm install --save-dev husky lint-staged

// package.json
{
  "husky": {
    "hooks": {
      "pre-commit": "lint-staged"
    }
  },
  "lint-staged": {
    "*.{ts,tsx}": [
      "eslint --fix",
      "prettier --write",
      "git add"
    ],
    "*.{ts,tsx}": [
      "bash -c 'npm test -- --findRelatedTests'"
    ]
  }
}

7. VS Code Extensions

Install these:

ESLint - Real-time linting
Prettier - Auto-formatting
Error Lens - Inline error messages
Code Spell Checker - Catch typos
GitLens - See git blame inline
SonarLint - Security and code smell detection

Settings:

// .vscode/settings.json
{
  "editor.codeActionsOnSave": {
    "source.fixAll.eslint": true
  },
  "editor.formatOnSave": true,
  "eslint.validate": [
    "javascript",
    "javascriptreact",
    "typescript",
    "typescriptreact"
  ]
}

8. Bundle Analyzer

Check bundle size impact:

npm install --save-dev webpack-bundle-analyzer

# Add to package.json
{
  "scripts": {
    "analyze": "webpack-bundle-analyzer stats.json"
  }
}

Run before submitting:

npm run build
npm run analyze

Flag if you've increased bundle size significantly.

The 5-Minute Self-Review Workflow

Here's my exact 5-minute process:

Minute 1: Quick Scan

# Check for obvious issues
git diff main | grep -i "console\."
git diff main | grep -i "TODO"
git diff main | grep -i "FIXME"
git diff main | grep -i "debugger"
git diff main | grep "\.only\|\.skip"

Minute 2: AI Review

# Let AI catch what you missed
git diff main | claude code --skill self-review > review.md
cat review.md

Fix critical issues flagged by AI.

Minute 3: Test & Lint

npm run lint
npm test
npm run build

All must pass.

Minute 4: Visual Review on GitHub

Create draft PR
Go to "Files changed"
Review each file as if you're the reviewer
Look for issues the tools missed

Minute 5: Final Checks

[ ] PR description complete
[ ] Screenshots added (if UI change)
[ ] Tests added
[ ] Documentation updated
[ ] Ready to convert to real PR

Reducing Review Cycles

Track Your Improvement

Week 1: Count review cycles per PR
Week 2: Implement self-review checklist
Week 3: Compare review cycles

My results:

Before: Average 3.2 review cycles per PR
After: Average 1.1 review cycles per PR
Time saved: ~2 days per PR

Make It a Habit

Triggers to start self-review:

Tests pass locally → Self-review
About to push → Self-review
Creating PR → Self-review draft first

Don't submit without self-review. Ever.

Common Objections

"I don't have time for self-review"

Self-review takes 10 minutes.
Each review cycle takes 4+ hours (waiting).
You'll save time.

"The reviewer will catch it anyway"

True, but:

They'll be annoyed
It wastes their time
It slows down your PR
It hurts your reputation

"I already reviewed it while coding"

No you didn't. You were focused on making it work.
Self-review is different - you're looking for issues.

Real Examples: Before and After Self-Review

Example 1: The Debugging Left Behind

Before self-review (would submit this):

function processOrder(order: Order) {
  console.log('Processing order:', order); // Debug
  console.log('User ID:', order.userId); // Debug

  const user = getUser(order.userId);
  console.log('User found:', user); // Debug

  if (!user) {
    console.error('User not found!'); // Debug
    return null;
  }

  console.log('Calculating total...'); // Debug
  const total = calculateTotal(order);
  console.log('Total:', total); // Debug

  return total;
}

After self-review:

function processOrder(order: Order): number | null {
  const user = getUser(order.userId);

  if (!user) {
    logger.error('User not found', { 
      orderId: order.id,
      userId: order.userId 
    });
    return null;
  }

  return calculateTotal(order);
}

Caught: 7 debug console.logs, no return type

Example 2: The Unhandled Edge Cases

Before self-review:

function getUserName(userId: string): string {
  const user = users.find(u => u.id === userId);
  return user.name;
}

After self-review:

function getUserName(userId: string): string {
  const user = users.find(u => u.id === userId);

  if (!user) {
    logger.warn('User not found', { userId });
    return 'Unknown User';
  }

  return user.name ?? 'Unnamed User';
}

Caught: No null check, no handling of missing name

Example 3: The Performance Issue

Before self-review:

function Component({ items }: { items: Item[] }) {
  const filtered = items.filter(i => i.active);
  const sorted = filtered.sort((a, b) => b.date - a.date);
  const formatted = sorted.map(i => formatItem(i));

  return <List items={formatted} />;
}

After self-review:

function Component({ items }: { items: Item[] }) {
  const processedItems = useMemo(
    () => items
      .filter(item => item.active)
      .sort((a, b) => b.date - a.date)
      .map(item => formatItem(item)),
    [items]
  );

  return <List items={processedItems} />;
}

Caught: Expensive operations running every render

Example 4: The Type Safety Disaster

Before self-review:

function processData(data: any) {
  const users = data.users;
  const posts = data.posts;

  return users.map(u => ({
    ...u,
    posts: posts.filter(p => p.userId === u.id)
  }));
}

After self-review:

interface User {
  id: string;
  name: string;
}

interface Post {
  id: string;
  userId: string;
  title: string;
}

interface UserWithPosts extends User {
  posts: Post[];
}

function processData(data: {
  users: User[];
  posts: Post[];
}): UserWithPosts[] {
  const { users, posts } = data;

  return users.map(user => ({
    ...user,
    posts: posts.filter(post => post.userId === user.id)
  }));
}

Caught: any type, no type safety, unclear return type

The Self-Review Mindset

Think Like a Reviewer

When self-reviewing, ask yourself:

"Would I approve this PR?"
"What questions would I ask?"
"What would confuse me?"
"What looks risky?"
"What could break?"

Be Honest With Yourself

Don't rationalize issues:

❌ "I'll fix that TODO later"
❌ "That console.log won't hurt"
❌ "Good enough for now"
❌ "The reviewer won't notice"

✅ "I should fix this now"
✅ "I should remove that"
✅ "I should make this clearer"
✅ "I should add a test"

Take Pride in Clean PRs

Your PR is a reflection of your professionalism.

Clean PRs show:

You care about quality
You respect reviewers' time
You think about maintainability
You're ready for senior roles

Measuring Success

Before Self-Review

Track for 2 weeks:

Review cycles per PR
Time from submit to approval
Number of "obvious" issues found by reviewers
Reviewer frustration (complaints about missed things)

After Self-Review

Track for 2 weeks:

Review cycles per PR (should drop 50%+)
Time from submit to approval (should drop 50%+)
"Obvious" issues (should drop 80%+)
Reviewer happiness (compliments on clean PRs)

My Results

Before:

Average review cycles: 3.2
Average time to approval: 2.3 days
Console.logs found by reviewers: 12/month
Test failures caught in review: 8/month

After:

Average review cycles: 1.1
Average time to approval: 0.5 days
Console.logs found by reviewers: 1/month
Test failures caught in review: 0/month

Time saved: ~40 hours/month

Conclusion

Self-review isn't extra work. It's the fastest way to ship code.

10 minutes of self-review saves:

✅ 2+ days of waiting
✅ Multiple review cycles
✅ Reviewer frustration
✅ Your reputation
✅ Embarrassing mistakes

The checklist:

Code quality (no console.logs, TODOs, etc.)
Type safety (no any, proper null handling)
React patterns (hooks, performance)
Testing (coverage, quality)
Security (no secrets, validation)
Accessibility (semantic HTML, ARIA)
Documentation (comments, README)
Git hygiene (clean commits)

The tools:

Claude Code skills (catches what you're blind to)
ESLint + TypeScript
Pre-commit hooks
AI review

The mindset:

Think like a reviewer
Be honest with yourself
Take pride in clean code

Start tomorrow:

Create draft PR
Review on GitHub
Run AI review: git diff main | claude code --skill self-review
Fix all issues
Submit clean PR
Get approved faster

Your reviewers will notice. Your career will benefit.

Self-review is the fastest path to senior engineer.

What's your self-review process? Share your checklist!

Copy-Paste and Go for PR descriptions

Tarun Moorjani — Fri, 06 Mar 2026 16:48:56 +0000

TypeScript PR Templates and Checklists: Copy-Paste and Go

Tarun Moorjani ・ Mar 6

#webdev #typescript #javascript #react

TypeScript PR Templates and Checklists: Copy-Paste and Go

Tarun Moorjani — Fri, 06 Mar 2026 02:14:53 +0000

I've reviewed over a lot TypeScript PRs. The best teams use templates and checklists. The struggling teams don't.

Without templates:

Developers forget to mention breaking changes
Type safety issues slip through
Performance regressions go unnoticed
Accessibility gets overlooked
PRs sit for days because reviewers need more context

With templates:

Every PR has consistent structure
Critical checks are never forgotten
Reviewers know exactly what to look for
Approval happens faster
Quality stays high

Here are the exact templates and checklists I use. Copy them. Customize them. Ship better code.

Quick Start: GitHub PR Template

Create .github/PULL_REQUEST_TEMPLATE.md in your repo:

## Changes

<!-- Brief description of what changed -->

## Type of Change

- [ ] 🐛 Bug fix (non-breaking change which fixes an issue)
- [ ] ✨ New feature (non-breaking change which adds functionality)
- [ ] 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] 📝 Documentation update
- [ ] 🎨 Style/UI update (no functional changes)
- [ ] ♻️ Code refactor (no functional changes)
- [ ] ⚡ Performance improvement
- [ ] ✅ Test update
- [ ] 🔧 Configuration change
- [ ] 📦 Dependency update

## Checklist

- [ ] My code follows the project's style guidelines
- [ ] I have performed a self-review of my code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Any dependent changes have been merged and published

## Related Issues

Fixes #
Related to #

This is the minimal viable template. Let's make it better.

The Complete TypeScript PR Template

## Summary

<!-- 1-2 sentence summary of changes -->

## Problem

<!-- What bug are you fixing? What feature are you adding? Why? -->

**Impact:**
<!-- How many users? How severe? Business context? -->

## Solution

<!-- High-level approach. Key decisions made and why. -->

**Type Safety:**
<!-- Any changes to types? New interfaces? Breaking type changes? -->

**Alternatives Considered:**
<!-- What else did you try? Why did you choose this approach? -->

## Type of Change

- [ ] 🐛 Bug fix (non-breaking)
- [ ] ✨ New feature (non-breaking)
- [ ] 💥 Breaking change
- [ ] 📝 Documentation
- [ ] ♻️ Refactor
- [ ] ⚡ Performance
- [ ] ✅ Tests
- [ ] 🔒 Security fix
- [ ] ♿ Accessibility improvement

## TypeScript Checklist

- [ ] No new `any` types (or justified with comments)
- [ ] No `@ts-ignore` without explanation
- [ ] All new interfaces/types are exported from appropriate files
- [ ] Type imports use `import type` syntax
- [ ] No type assertions without runtime validation
- [ ] Discriminated unions used for state machines
- [ ] Generic types have proper constraints
- [ ] Return types are explicit for public APIs

## Testing

**Manual Testing:**
- [ ] Tested happy path
- [ ] Tested error cases
- [ ] Tested edge cases
- [ ] Verified in dev environment
- [ ] Verified in staging (if applicable)

**Automated Testing:**
- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] E2E tests added/updated (if applicable)
- [ ] All tests passing locally
- [ ] Coverage maintained or improved

**Test Cases:**
<!-- List specific scenarios tested -->

## Performance

- [ ] No performance regressions
- [ ] Expensive operations are memoized
- [ ] No unnecessary re-renders
- [ ] Bundle size impact checked (if applicable)
- [ ] Large imports are code-split
- [ ] Images are optimized (if applicable)

## Security

- [ ] No sensitive data in logs
- [ ] No hardcoded secrets
- [ ] User input is validated/sanitized
- [ ] No XSS vulnerabilities
- [ ] Authentication/authorization checked
- [ ] CSRF protection in place (if applicable)

## Accessibility

- [ ] Semantic HTML used
- [ ] ARIA labels added where needed
- [ ] Keyboard navigation works
- [ ] Focus management is correct
- [ ] Color contrast is sufficient
- [ ] Screen reader tested (if UI change)

## Breaking Changes

<!-- List any breaking changes and migration path -->

**Migration Guide:**
<!-- How do other developers/users adapt to this change? -->

## Screenshots/Videos

<!-- For UI changes: before/after screenshots or screen recording -->

## Deployment Notes

- [ ] No database migrations required
- [ ] No environment variable changes required
- [ ] No infrastructure changes required
- [ ] Feature flags configured (if applicable)
- [ ] Rollback plan documented

**Rollback Plan:**
<!-- How to rollback if issues occur -->

## Related

Fixes #
Closes #
Part of #
Depends on #
Blocks #

**Documentation:**
- [ ] README updated
- [ ] API docs updated
- [ ] Changelog updated
- [ ] Migration guide created (if breaking)

## Reviewer Notes

<!-- Anything specific reviewers should focus on? Questions for reviewers? -->

---

## Pre-Merge Checklist

- [ ] CI/CD pipeline passing
- [ ] Code reviewed and approved
- [ ] All comments addressed
- [ ] Conflicts resolved
- [ ] Branch is up to date with main/master
- [ ] Ready to merge

Template Variations by PR Type

1. Bug Fix Template

## Bug Fix: [Brief Description]

## Bug Details

**What's Broken:**
<!-- Exact error message, unexpected behavior -->

**Steps to Reproduce:**
1. 
2. 
3. 

**Expected Behavior:**
<!-- What should happen -->

**Actual Behavior:**
<!-- What actually happens -->

**Impact:**
- Severity: [ ] Critical [ ] High [ ] Medium [ ] Low
- Affected Users: 
- Frequency: 
- Support Tickets: 

## Root Cause

<!-- Technical explanation of why this bug exists -->

## Solution

<!-- How does your fix solve the problem? -->

**Code Changes:**
- File 1: [what changed]
- File 2: [what changed]

## Testing

**Reproduction:**
- [ ] Reproduced bug before fix
- [ ] Verified fix resolves issue
- [ ] Cannot reproduce bug after fix

**Regression Testing:**
- [ ] Related features still work
- [ ] Edge cases covered
- [ ] Added regression test

**Test Cases:**
1. Original bug scenario: ✅ Fixed
2. Edge case A: ✅ Handled
3. Edge case B: ✅ Handled

## Type Safety

- [ ] No new `any` types
- [ ] Null/undefined handling improved
- [ ] Type guards added where needed

## Risks

**Rollback Plan:**
<!-- How to rollback if this causes issues -->

**Monitoring:**
<!-- What metrics to watch after deployment -->

Fixes #[issue-number]

2. New Feature Template

## Feature: [Feature Name]

## User Story

**As a** [type of user]
**I want** [goal]
**So that** [benefit]

## Business Value

**Why This Feature:**
<!-- Business justification, user need, competitive advantage -->

**Success Metrics:**
- Metric 1: 
- Metric 2: 
- Metric 3: 

## Implementation

**User Flow:**
1. User does X
2. System does Y
3. User sees Z

**Technical Approach:**
<!-- High-level architecture, key components -->

**TypeScript Patterns Used:**
- [ ] Discriminated unions for state
- [ ] Generic components
- [ ] Type-safe API calls
- [ ] Proper error types

## API Changes

**New Endpoints:**
<!-- If backend changes -->

**New Types:**

typescript
// List new interfaces/types


**Breaking Changes:**
<!-- Any breaking changes in API or types -->

## UI/UX Changes

**Screenshots:**
<!-- Before/after if modifying existing feature -->
<!-- New UI screenshots -->

**Responsive Design:**
- [ ] Mobile tested
- [ ] Tablet tested
- [ ] Desktop tested

**Accessibility:**
- [ ] Keyboard navigation works
- [ ] Screen reader compatible
- [ ] ARIA labels added

## Testing

**Feature Testing:**
- [ ] Happy path works
- [ ] Error handling works
- [ ] Edge cases handled

**Integration Testing:**
- [ ] Works with existing features
- [ ] No regressions in related features

**Performance:**
- [ ] No performance degradation
- [ ] Optimized for large datasets (if applicable)

## Documentation

- [ ] User documentation added
- [ ] API documentation added
- [ ] Code comments added
- [ ] README updated

## Rollout Plan

**Feature Flag:**
<!-- Feature flag name and configuration -->

**Phased Rollout:**
- [ ] Internal testing
- [ ] Beta users
- [ ] 10% of users
- [ ] 50% of users
- [ ] 100% of users

Implements #[issue-number]

3. Refactoring Template

## Refactor: [What You're Refactoring]

## Motivation

**Problems with Current Code:**
1. 
2. 
3. 

**Why Refactor Now:**
<!-- Why is this worth doing right now? -->

## Changes

**Before:**

typescript
// Old pattern/code


**After:**

typescript
// New pattern/code


**Improvements:**
- [ ] Reduced complexity
- [ ] Improved type safety
- [ ] Better performance
- [ ] Easier to test
- [ ] More maintainable

## TypeScript Improvements

- [ ] Replaced `any` with proper types
- [ ] Added type guards
- [ ] Improved generic constraints
- [ ] Better error types
- [ ] Stricter null checks

## Impact Analysis

**Files Changed:** 
**Lines Added/Removed:** 
**Behavior Changes:** None (pure refactor)

## Testing

- [ ] All existing tests pass
- [ ] No new bugs introduced
- [ ] Performance is same or better
- [ ] Coverage maintained or improved

**Verification:**
- [ ] Ran full test suite
- [ ] Manual smoke testing
- [ ] Verified in staging

## Migration Path

**For Other Developers:**
<!-- How do teammates adapt their code? -->

**Deprecated Code:**
<!-- What's being removed? When? -->

**Examples:**

typescript
// Old way (still works, deprecated)
const data = oldFunction();

// New way (preferred)
const data = newFunction();


## Risks

**Risk Level:** [ ] Low [ ] Medium [ ] High

**Mitigation:**
<!-- How are you reducing risk? -->

**Rollback:**
<!-- How to rollback if needed -->

Related to #[issue-number]

4. TypeScript Upgrade Template

## TypeScript Upgrade: [Old Version] → [New Version]

## Motivation

**Why Upgrade:**
- [ ] Security fixes
- [ ] New features needed
- [ ] Better type inference
- [ ] Bug fixes
- [ ] EOL of current version

**Benefits:**
1. 
2. 
3. 

## Breaking Changes

**Compiler:**
<!-- New errors, changed behavior -->

**Type System:**
<!-- Type inference changes, new strictness -->

**List of Breaking Changes:**
1. Change 1: Impact and fix
2. Change 2: Impact and fix
3. Change 3: Impact and fix

## Migration Summary

**Files Modified:** 
**Type Errors Fixed:** 
**Deprecations Addressed:** 

**Categories of Changes:**
- [ ] Added null checks
- [ ] Fixed type assertions
- [ ] Updated type imports
- [ ] Fixed generic constraints
- [ ] Other: 

## Testing

- [ ] All tests passing
- [ ] No new type errors
- [ ] No runtime errors in dev
- [ ] No console warnings
- [ ] Verified in staging

**Compatibility:**
- [ ] No breaking changes for consumers
- [ ] Library types updated
- [ ] Type definitions published (if library)

## Rollback Plan

<!-- How to downgrade if issues arise -->

## Developer Impact

**Action Required:**
- [ ] None (fully backward compatible)
- [ ] Update local TypeScript: `npm install`
- [ ] Fix new type errors (list provided)
- [ ] Update IDE settings

**Communication:**
<!-- How will you notify the team? -->

Related to #[issue-number]

5. Dependency Update Template

## Dependency Update: [Package] [Old] → [New]

## Package Details

**Package:** 
**Current Version:** 
**New Version:** 
**Type:** [ ] Production [ ] Development

## Why Update

- [ ] Security vulnerability (CVE: )
- [ ] Bug fix
- [ ] New features needed
- [ ] Breaking changes in ecosystem
- [ ] Performance improvement
- [ ] Other: 

## Changes

**Breaking Changes:**
<!-- List breaking changes from changelog -->

**New Features:**
<!-- Relevant new features -->

**Bug Fixes:**
<!-- Relevant bug fixes -->

## TypeScript Impact

- [ ] Type definitions updated
- [ ] No new type errors
- [ ] API changes reflected in types
- [ ] Generic constraints updated (if applicable)

## Testing

- [ ] All tests passing
- [ ] Verified functionality unchanged
- [ ] Tested new features (if using them)
- [ ] No console warnings
- [ ] Bundle size checked

**Bundle Size:**
- Before: 
- After: 
- Delta: 

## Migration Notes

**Code Changes Required:**
<!-- None, or list changes -->

**Configuration Changes:**
<!-- None, or list changes -->

## Compatibility

- [ ] Node.js version compatible
- [ ] React version compatible
- [ ] Other dependencies compatible
- [ ] No peer dependency conflicts

## Rollback

**Rollback Command:**

bash
npm install [package]@[old-version]


**Risk:** [ ] Low [ ] Medium [ ] High

Fixes #[issue-number]

6. Performance Optimization Template

## Performance: [What You're Optimizing]

## Problem

**Current Performance:**
- Metric 1: 
- Metric 2: 
- Metric 3: 

**User Impact:**
<!-- How does slow performance affect users? -->

## Solution

**Optimization Strategy:**
1. 
2. 
3. 

**Changes:**
- [ ] Added memoization
- [ ] Implemented code splitting
- [ ] Optimized re-renders
- [ ] Reduced bundle size
- [ ] Improved algorithm complexity
- [ ] Other: 

## Performance Gains

**Before:**
- Load time: 
- Bundle size: 
- Memory usage: 
- Render time: 

**After:**
- Load time: 
- Bundle size: 
- Memory usage: 
- Render time: 

**Improvement:** 

## TypeScript Considerations

- [ ] No type safety sacrificed for performance
- [ ] Memoization properly typed
- [ ] Generic constraints maintained
- [ ] Type inference still works

## Testing

**Performance Testing:**
- [ ] Lighthouse score improved
- [ ] Bundle analyzer checked
- [ ] Profiler shows improvements
- [ ] Tested with large datasets

**Regression Testing:**
- [ ] All features still work
- [ ] No visual regressions
- [ ] Error handling unchanged

## Benchmarks

Scenario 1:

Before: XXms
After: XXms
Improvement: XX%

Scenario 2:

Before: XXms
After: XXms
Improvement: XX%


## Trade-offs

**What was sacrificed:**
<!-- Code complexity? Readability? -->

**Is it worth it:**
<!-- Justify the trade-offs -->

Fixes #[issue-number]

7. Security Fix Template

## Security Fix: [Brief Description]

## ⚠️ Security Issue

**Severity:** [ ] Critical [ ] High [ ] Medium [ ] Low

**Type:**
- [ ] XSS vulnerability
- [ ] SQL injection
- [ ] Authentication bypass
- [ ] Authorization issue
- [ ] Data exposure
- [ ] Dependency vulnerability
- [ ] Other: 

**Affected:**
<!-- What users/data/systems are affected? -->

## Vulnerability Details

<!-- Describe the vulnerability (avoid public disclosure of exploits) -->

**CVE:** (if applicable)
**CVSS Score:** (if applicable)

## Fix

**What Changed:**
<!-- High-level description -->

**Security Improvements:**
- [ ] Input validation added
- [ ] Output encoding added
- [ ] Authentication strengthened
- [ ] Authorization checks added
- [ ] Dependency updated
- [ ] Other: 

## TypeScript Safety

- [ ] Type guards prevent injection
- [ ] Branded types for validated data
- [ ] Type-safe validation schemas
- [ ] No `any` in security-critical code

## Testing

**Security Testing:**
- [ ] Attempted exploit before fix (fails)
- [ ] Attempted exploit after fix (blocked)
- [ ] Penetration testing performed
- [ ] Security scan passed

**Regression Testing:**
- [ ] All features still work
- [ ] No broken authentication
- [ ] No broken authorization

## Deployment

**Urgency:** [ ] Deploy Immediately [ ] Deploy Soon [ ] Regular Deployment

**Rollout:**
<!-- How will this be deployed? -->

**Monitoring:**
<!-- What to watch after deployment? -->

## Communication

- [ ] Security team notified
- [ ] Users notified (if needed)
- [ ] Security advisory published (if needed)

Fixes #[issue-number]

Specialized Checklists

Type Safety Checklist

## Type Safety Review

**Basic Type Safety:**
- [ ] No explicit `any` types (or justified)
- [ ] No implicit `any` from missing types
- [ ] No `@ts-ignore` comments (or explained)
- [ ] No `@ts-expect-error` without TODO
- [ ] Type assertions justified with comments

**Advanced Type Safety:**
- [ ] Discriminated unions for state machines
- [ ] Type guards for runtime validation
- [ ] Generic constraints properly defined
- [ ] Conditional types used correctly
- [ ] Template literal types where appropriate

**Import/Export:**
- [ ] Type imports use `import type`
- [ ] All public APIs have explicit types
- [ ] Types exported from appropriate files
- [ ] No circular type dependencies

**Null/Undefined:**
- [ ] Null checks before access
- [ ] Optional chaining used (`?.`)
- [ ] Nullish coalescing used (`??`)
- [ ] Non-null assertions justified (`!`)

**Functions:**
- [ ] Parameters properly typed
- [ ] Return types explicit for public APIs
- [ ] Async functions return Promise<T>
- [ ] Callbacks properly typed

**React Components:**
- [ ] Props interface defined
- [ ] Children typed correctly
- [ ] Event handlers typed
- [ ] Ref types correct
- [ ] Generic components properly constrained

**API/External Data:**
- [ ] API responses validated at runtime
- [ ] JSON parsing is type-safe
- [ ] External data has type guards
- [ ] Errors are properly typed

**Edge Cases:**
- [ ] Empty arrays handled
- [ ] undefined vs null distinguished
- [ ] Optional properties handled
- [ ] Union types narrowed correctly

Performance Checklist

## Performance Review

**React Performance:**
- [ ] No unnecessary re-renders
- [ ] useMemo for expensive calculations
- [ ] useCallback for callbacks
- [ ] React.memo for expensive components
- [ ] Virtualization for large lists
- [ ] Code splitting implemented

**Bundle Size:**
- [ ] Tree shaking enabled
- [ ] Dead code eliminated
- [ ] Large libraries code-split
- [ ] Import only what's needed
- [ ] Bundle analyzer reviewed

**Assets:**
- [ ] Images optimized
- [ ] SVGs minified
- [ ] Fonts subset (if applicable)
- [ ] Lazy loading for images
- [ ] WebP format used (if applicable)

**Network:**
- [ ] API calls debounced/throttled
- [ ] Requests cached appropriately
- [ ] Parallel requests where possible
- [ ] Request cancellation on unmount
- [ ] Prefetching implemented

**Runtime:**
- [ ] No synchronous expensive operations
- [ ] Web Workers for heavy computation
- [ ] RequestIdleCallback for non-critical
- [ ] Intersection Observer for lazy load
- [ ] No memory leaks (listeners cleaned up)

**Measurement:**
- [ ] Lighthouse score checked
- [ ] Bundle size measured
- [ ] Core Web Vitals acceptable
- [ ] Performance budget not exceeded

Accessibility Checklist

## Accessibility Review

**Semantic HTML:**
- [ ] Proper heading hierarchy (h1-h6)
- [ ] Lists use ul/ol/li
- [ ] Buttons are <button>
- [ ] Links are <a>
- [ ] Forms use <form>
- [ ] Tables use proper structure

**ARIA:**
- [ ] ARIA labels where needed
- [ ] ARIA roles appropriate
- [ ] ARIA states/properties correct
- [ ] No redundant ARIA
- [ ] Live regions for dynamic content

**Keyboard Navigation:**
- [ ] All interactive elements focusable
- [ ] Tab order logical
- [ ] Focus visible
- [ ] No keyboard traps
- [ ] Shortcuts don't conflict
- [ ] Enter/Space work on custom buttons

**Screen Reader:**
- [ ] Alt text for images
- [ ] Form labels associated
- [ ] Error messages announced
- [ ] Loading states announced
- [ ] Success messages announced

**Visual:**
- [ ] Color contrast meets WCAG AA
- [ ] Information not conveyed by color alone
- [ ] Focus indicators visible
- [ ] Text is resizable
- [ ] No flashing/blinking content

**Forms:**
- [ ] Labels properly associated
- [ ] Required fields marked
- [ ] Error messages clear
- [ ] Error messages linked to fields
- [ ] Field validation accessible

**Testing:**
- [ ] Keyboard-only navigation tested
- [ ] Screen reader tested (if major change)
- [ ] Axe DevTools passing
- [ ] Lighthouse accessibility > 90

Security Checklist

## Security Review

**Input Validation:**
- [ ] All user input validated
- [ ] Type validation with TypeScript
- [ ] Runtime validation (Zod/Yup)
- [ ] SQL injection prevented
- [ ] Command injection prevented

**Output Encoding:**
- [ ] XSS prevention (React does this, but check dangerouslySetInnerHTML)
- [ ] No unsanitized HTML rendering
- [ ] URLs validated before rendering
- [ ] JavaScript: URLs blocked

**Authentication:**
- [ ] Authentication required where needed
- [ ] Tokens stored securely
- [ ] Session management secure
- [ ] Logout clears all auth data
- [ ] Password fields use type="password"

**Authorization:**
- [ ] Authorization checks on sensitive operations
- [ ] RBAC implemented correctly
- [ ] No client-side only authorization
- [ ] API authorization verified

**Data Exposure:**
- [ ] No sensitive data in logs
- [ ] No sensitive data in URLs
- [ ] No sensitive data in error messages
- [ ] No API keys in frontend code
- [ ] Environment variables used correctly

**Dependencies:**
- [ ] No known vulnerabilities (npm audit)
- [ ] Dependencies up to date
- [ ] Only trusted packages used
- [ ] Package checksums verified

**HTTPS/Transport:**
- [ ] All requests use HTTPS
- [ ] Cookies are Secure and HttpOnly
- [ ] CORS configured correctly
- [ ] CSP headers configured

**File Uploads:**
- [ ] File type validation
- [ ] File size limits
- [ ] Virus scanning (if applicable)
- [ ] Files not executable on server

Pre-Submission Checklist

Use this before clicking "Create Pull Request":

## Pre-Submission Checklist

**Code Quality:**
- [ ] Code is formatted (Prettier)
- [ ] Code is linted (ESLint)
- [ ] No console.logs left in code
- [ ] No commented-out code
- [ ] No TODO comments (or issues created)
- [ ] No debug code left in

**Git:**
- [ ] Commits are logical
- [ ] Commit messages are clear
- [ ] No merge commits (rebased)
- [ ] No conflicts with base branch
- [ ] Branch is up to date

**Testing:**
- [ ] All tests passing locally
- [ ] New tests added for new code
- [ ] Coverage maintained or improved
- [ ] Manual testing performed
- [ ] Edge cases tested

**Documentation:**
- [ ] README updated (if needed)
- [ ] Code comments added
- [ ] Complex logic explained
- [ ] API documentation updated
- [ ] Migration guide written (if breaking)

**TypeScript:**
- [ ] No new type errors
- [ ] No new `any` types
- [ ] Type imports use `import type`
- [ ] Public APIs have explicit types

**Performance:**
- [ ] No performance regressions
- [ ] Bundle size checked
- [ ] Expensive operations optimized
- [ ] Images optimized (if added)

**Security:**
- [ ] No hardcoded secrets
- [ ] Input validation added
- [ ] No XSS vulnerabilities
- [ ] npm audit passing

**Accessibility:**
- [ ] Keyboard navigation works
- [ ] ARIA labels added
- [ ] Color contrast sufficient
- [ ] Screen reader tested (if major UI)

**CI/CD:**
- [ ] CI pipeline passing
- [ ] Build succeeds
- [ ] Deploy previewed (if available)

**PR Description:**
- [ ] Problem clearly described
- [ ] Solution explained
- [ ] Testing documented
- [ ] Screenshots added (if UI)
- [ ] Links to issues added
- [ ] Breaking changes noted

Reviewer Checklist

For the person reviewing the PR:

## Code Review Checklist

**First Pass (5 minutes):**
- [ ] PR description is clear
- [ ] Changes make sense at high level
- [ ] Tests are included
- [ ] CI is passing
- [ ] No obvious red flags

**Type Safety (10 minutes):**
- [ ] No liberal use of `any`
- [ ] Type assertions justified
- [ ] Null checks present
- [ ] Proper type narrowing
- [ ] Generic constraints correct

**Code Quality (15 minutes):**
- [ ] Code is readable
- [ ] Logic is sound
- [ ] No obvious bugs
- [ ] Error handling present
- [ ] Edge cases handled

**React Patterns (10 minutes):**
- [ ] Hooks rules followed
- [ ] Dependencies correct
- [ ] No unnecessary re-renders
- [ ] State management appropriate
- [ ] No prop drilling (or justified)

**Testing (5 minutes):**
- [ ] Tests are meaningful
- [ ] Coverage is adequate
- [ ] Edge cases tested
- [ ] Mocks are appropriate

**Performance (5 minutes):**
- [ ] No obvious performance issues
- [ ] Expensive operations optimized
- [ ] Bundle size impact acceptable
- [ ] Images optimized (if added)

**Security (5 minutes):**
- [ ] No security vulnerabilities
- [ ] Input validated
- [ ] No exposed secrets
- [ ] Authentication/authorization correct

**Accessibility (5 minutes):**
- [ ] Semantic HTML used
- [ ] Keyboard navigation works
- [ ] ARIA labels present
- [ ] Color contrast good

**Final Check:**
- [ ] Would I be comfortable shipping this?
- [ ] Is this better than it was?
- [ ] Are my concerns addressed?
- [ ] Ready to approve?

GitHub Setup

Install PR Template

# Create .github directory
mkdir -p .github

# Create PR template
touch .github/PULL_REQUEST_TEMPLATE.md

Paste your chosen template into PULL_REQUEST_TEMPLATE.md.

Multiple Templates

For different PR types:

.github/
  PULL_REQUEST_TEMPLATE/
    bug_fix.md
    feature.md
    refactor.md
    dependency.md

Users select template when creating PR.

Issue Templates

.github/
  ISSUE_TEMPLATE/
    bug_report.md
    feature_request.md

AI Tools: Automate Template Generation

Don't fill out templates manually. Let AI do the heavy lifting while you add context.

Claude Code Skills: The Secret to Consistency

Claude Code skills are game-changing for PR templates. They enforce consistency across your entire team automatically.

What are Claude Code skills?

Reusable prompts that execute consistently
Live in your .claude/ directory
Version controlled with your code
Shared across your team

The power: Same template, same quality, every time.

Setting Up PR Template Skills

Step 1: Create a skill file

mkdir -p .claude/skills
touch .claude/skills/pr-description.md

Step 2: Write the skill

# PR Description Generator

You are helping a developer write a TypeScript PR description.

## Your Task

Analyze the git diff and generate a complete PR description following our template.

## Template to Use

## Summary
[1-2 sentence overview]

## Problem
[What bug/feature? Why does it matter?]

**Impact:**
[Users affected, severity, business context]

## Solution
[High-level approach, key decisions]

**Type Safety:**
[Any type changes? Breaking changes?]

## Testing
**Manual Testing:**
- [ ] Happy path tested
- [ ] Error cases tested
- [ ] Edge cases tested

**Automated Testing:**
- [ ] Unit tests added
- [ ] Integration tests added
- [ ] All tests passing

## TypeScript Checklist
- [ ] No new `any` types (or justified)
- [ ] No `@ts-ignore` without explanation
- [ ] Type imports use `import type`
- [ ] Proper null handling

## Performance
- [ ] No performance regressions
- [ ] Expensive operations memoized
- [ ] Bundle size checked

## Security
- [ ] No sensitive data in logs
- [ ] Input validated
- [ ] No XSS vulnerabilities

## Screenshots
[If UI change]

Fixes #

## Instructions

1. Read the git diff carefully
2. Identify what changed and why
3. Fill out each section of the template
4. Be specific - no vague statements
5. Check all relevant checklist items based on the changes
6. Leave unchecked items that don't apply
7. If you don't have enough context for a section, put [NEEDS AUTHOR INPUT]

## Important

- Focus on WHY, not just WHAT changed
- Identify type safety implications
- Flag potential performance impacts
- Note breaking changes
- Suggest test cases based on code changes

Step 3: Use the skill

# In your terminal
git diff main > /tmp/diff.txt
claude code --skill pr-description --input /tmp/diff.txt

Or even better:

# Create an alias
alias pr-gen="git diff main | claude code --skill pr-description"

# Now just run:
pr-gen

Result: Consistent, high-quality PR descriptions every time.

Creating Team-Specific Skills

Bug Fix Skill:

# .claude/skills/pr-bug-fix.md

Generate a bug fix PR description.

## Template

## Bug Fix: [Title]

## Bug Details

**What's Broken:**
[Error message, unexpected behavior]

**Steps to Reproduce:**
1. 
2. 
3. 

**Impact:**
- Severity: [Critical/High/Medium/Low]
- Users affected: [number or percentage]
- Frequency: [always/often/sometimes/rare]

## Root Cause
[Technical explanation]

## Solution
[How the fix works]

## Testing
**Verification:**
- [ ] Reproduced bug before fix
- [ ] Bug no longer reproduces
- [ ] Regression test added

Fixes #

## Instructions

1. Analyze the diff
2. Identify what was broken and why
3. Explain the fix clearly
4. Suggest appropriate test cases
5. Mark severity based on code impact

Feature Skill:

# .claude/skills/pr-feature.md

Generate a feature PR description.

## Template

## Feature: [Name]

## User Story
**As a** [user type]
**I want** [goal]
**So that** [benefit]

## Business Value
[Why this feature matters]

## Implementation
[Technical approach]

**New Types:**

typescript
// List interfaces/types added


## Testing
- [ ] Feature works end-to-end
- [ ] Error handling tested
- [ ] Edge cases covered

## Screenshots
[Before/after or new UI]

Implements #

## Instructions

1. Identify the new feature from the diff
2. Infer user story from code
3. List new TypeScript types/interfaces
4. Suggest comprehensive test cases
5. Request screenshots if UI changes detected

Refactoring Skill:

# .claude/skills/pr-refactor.md

Generate a refactoring PR description.

## Template

## Refactor: [What]

## Motivation
**Problems with current code:**
1. 
2. 

**Why now:**
[Why is this worth doing?]

## Changes
**Before:**

typescript
// Old pattern


**After:**

typescript
// New pattern


**Improvements:**
- [ ] Reduced complexity
- [ ] Better type safety
- [ ] Improved performance
- [ ] More testable

## Testing
- [ ] All existing tests pass
- [ ] No behavior changes
- [ ] Performance same or better

Related to #

## Instructions

1. Compare before/after patterns
2. Identify improvements (complexity, types, performance)
3. Ensure NO behavior changes mentioned
4. Verify tests validate refactor safety

Advanced: Checklist Validator Skill

# .claude/skills/pr-checklist-validator.md

You are a PR checklist validator for TypeScript React projects.

## Your Task

Review the code changes and verify each checklist item.

## Checklist to Validate

**Type Safety:**
- No explicit `any` types (or justified with comments)
- No `@ts-ignore` without explanation
- Type imports use `import type` syntax
- Proper null/undefined handling
- Type assertions are justified

**React Patterns:**
- Hooks rules followed (no conditional hooks)
- useEffect dependencies are correct
- No unnecessary re-renders
- Props are properly typed

**Performance:**
- Expensive operations are memoized
- No large imports (use tree-shaking)
- Images are optimized
- Code splitting where appropriate

**Security:**
- No hardcoded secrets
- Input is validated
- No XSS vulnerabilities
- No sensitive data in logs

**Accessibility:**
- Semantic HTML used
- ARIA labels where needed
- Keyboard navigation works

## Instructions

1. Analyze the diff carefully
2. For each checklist category:
   - Mark ✅ if ALL items in that category pass
   - Mark ⚠️ if some items need attention
   - Mark ❌ if critical items are violated
3. For any ⚠️ or ❌, list specific issues with file/line numbers
4. Provide concrete suggestions for fixes

## Output Format

### Type Safety ✅
All type safety checks pass.

### React Patterns ⚠️
Issues found:
- `UserProfile.tsx:45` - useEffect missing `userId` dependency
- `Dashboard.tsx:123` - useMemo needed for expensive calculation

### Performance ✅
No performance concerns.

### Security ❌
Critical issues:
- `api.ts:34` - API key hardcoded in source
- `LoginForm.tsx:67` - Password logged to console

### Accessibility ⚠️
Issues found:
- `Modal.tsx:23` - Missing ARIA label on close button

Usage:

git diff main | claude code --skill pr-checklist-validator

GitHub Copilot for Quick Templates

Copilot is great for quick, lightweight PR descriptions.

In VS Code, trigger Copilot Chat:

@workspace Generate a PR description for my staged changes. 
Use our team template: Problem, Solution, Testing, Type Safety checks.

Copilot strengths:

✅ Fast (inline in IDE)
✅ Understands workspace context
✅ Good for small PRs
✅ Integrated with GitHub PR UI

Copilot limitations:

❌ Less customizable than skills
❌ Shorter context window
❌ Can't enforce team standards as strictly
❌ Inconsistent output quality

Best use: Quick bug fixes and small features.

Claude (Web/API) for Complex PRs

For large, complex PRs, use Claude directly.

Example prompt:

I need a comprehensive PR description. Here's my git diff:

[paste diff]

Please create a PR description with:

1. Problem section (why this change is needed, business impact)
2. Solution section (technical approach, key decisions, why we chose this)
3. Testing section (specific test cases, coverage)
4. Type Safety review (any new types, breaking changes)
5. Performance impact (bundle size, runtime performance)
6. Security considerations
7. Breaking changes (if any, with migration guide)

Our team standards:
- Be specific, not vague
- Include actual numbers (users affected, performance gains)
- List test cases explicitly
- Flag all breaking changes
- Note TypeScript-specific concerns

Make it thorough and professional.

Claude strengths:

✅ Handles large diffs (100KB+)
✅ Deep analysis
✅ Can iterate and refine
✅ Follows complex instructions

Best use: Large refactors, complex features, critical fixes.

The Hybrid Workflow (Recommended)

Combine tools for best results:

# 1. Generate base description with skill
git diff main | claude code --skill pr-description > pr.md

# 2. Validate with checklist skill
git diff main | claude code --skill pr-checklist-validator >> pr.md

# 3. Review and add context
# Open pr.md, add:
# - Business context
# - Impact numbers
# - Screenshots
# - Specific testing details

# 4. Polish with Claude
# Paste pr.md into Claude: "Make this more concise and professional"

# 5. Copy to GitHub PR

Time savings:

Manual: 15-20 minutes
With skills: 5 minutes
With skills + AI polish: 7 minutes

Quality improvement:

Consistent structure ✅
All sections filled ✅
Checklist validated ✅
Professional tone ✅

Creating a Skills Library

Build a collection of reusable skills:

.claude/skills/
├── pr-description.md          # General PR template
├── pr-bug-fix.md             # Bug fix specific
├── pr-feature.md             # New feature specific
├── pr-refactor.md            # Refactoring specific
├── pr-dependency.md          # Dependency update
├── pr-performance.md         # Performance optimization
├── pr-security.md            # Security fix
├── pr-checklist-validator.md # Validate against checklist
├── pr-type-safety-check.md   # Type safety audit
└── pr-breaking-changes.md    # Breaking change detector

Team setup:

# In your README.md
## PR Description Generation

We use Claude Code skills for consistent PR descriptions.

**Quick start:**

bash

Generate PR description

git diff main | claude code --skill pr-description

Validate checklist

git diff main | claude code --skill pr-checklist-validator


**Available skills:**
- `pr-description` - General purpose
- `pr-bug-fix` - Bug fixes
- `pr-feature` - New features
- `pr-refactor` - Refactoring
- `pr-checklist-validator` - Validate against checklist

Example: Complete Skill-Driven Workflow

Scenario: You just fixed a bug.

# 1. Check your changes
git diff main

# 2. Generate bug fix description
git diff main | claude code --skill pr-bug-fix > pr-description.md

# 3. Validate against checklist
git diff main | claude code --skill pr-checklist-validator >> pr-description.md

# 4. Check for breaking changes
git diff main | claude code --skill pr-breaking-changes >> pr-description.md

# 5. Review the generated description
cat pr-description.md

# 6. Add your context:
# - Actual user impact numbers
# - Screenshots
# - Specific testing you performed

# 7. Create PR
gh pr create --title "Fix: User profile crash" --body-file pr-description.md

Result: Professional, complete PR description in under 5 minutes.

Advanced: Type-Safe Checklist Validator

Create a skill that not only validates but generates TypeScript-specific feedback:

# .claude/skills/pr-type-safety-deep-check.md

You are a TypeScript expert reviewing a PR for type safety.

## Deep Type Safety Analysis

Analyze the diff and report on:

**1. Any Usage:**
- Count of explicit `any` types
- Count of implicit `any` (missing types)
- For each `any`, suggest proper type

**2. Type Assertions:**
- List all `as` assertions
- Flag dangerous assertions
- Suggest type guards where appropriate

**3. Null Safety:**
- Find all optional chaining (`?.`) uses
- Find all nullish coalescing (`??`) uses
- Find missing null checks
- Suggest improvements

**4. Generic Types:**
- List all generic functions/components
- Check if constraints are needed
- Verify type inference works correctly

**5. Discriminated Unions:**
- Identify state machines
- Check if discriminated unions are used
- Suggest union types for boolean soup

**6. Type Imports:**
- Count `import type` vs regular imports
- Suggest converting to `import type`

## Output Format

### Type Safety Score: X/10

**Issues Found:**

🔴 **Critical (must fix):**
- Line 45: `any` type without justification
- Line 67: Unsafe type assertion

🟡 **Warnings (should fix):**
- Line 123: Missing null check
- Line 145: Could use discriminated union

🟢 **Suggestions (nice to have):**
- Line 200: Convert to `import type`
- Line 234: Add generic constraint

**Detailed Analysis:**

[For each issue, provide:
- File and line number
- Current code
- Suggested fix
- Explanation why]

Integration with GitHub Actions

Automate skill execution in CI:

# .github/workflows/pr-check.yml
name: PR Quality Check

on: [pull_request]

jobs:
  pr-quality:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3

      - name: Install Claude Code
        run: npm install -g @anthropic-ai/claude-code

      - name: Generate diff
        run: git diff origin/main > /tmp/pr-diff.txt

      - name: Validate checklist
        run: |
          claude code --skill pr-checklist-validator \
            --input /tmp/pr-diff.txt > /tmp/validation.md

      - name: Post validation as comment
        uses: actions/github-script@v6
        with:
          script: |
            const fs = require('fs');
            const validation = fs.readFileSync('/tmp/validation.md', 'utf8');

            github.rest.issues.createComment({
              issue_number: context.issue.number,
              owner: context.repo.owner,
              repo: context.repo.repo,
              body: `## Automated PR Validation\n\n${validation}`
            });

Skills vs. Manual Templates: Comparison

Aspect	Manual Template	GitHub Copilot	Claude Code Skills
Consistency	❌ Varies by person	🟡 Somewhat	✅ Exactly the same
Time to fill	15 min	5 min	2 min
Quality	🟡 Depends on author	🟡 Pretty good	✅ Excellent
Team standards	❌ Manual enforcement	❌ Hard to enforce	✅ Automatic
Customization	✅ Fully custom	🟡 Limited	✅ Fully custom
Version control	✅ Yes	❌ No	✅ Yes
Onboarding	Slow (learn template)	Medium	Fast (just run)

Verdict: Claude Code skills win for team consistency and quality.

Real-World Example: Before and After

Before (manual template):

Developer spends 20 minutes filling template
Forgets to check type safety items
Misses performance implications
Generic testing section ("tested it, works")
Inconsistent quality across team

After (Claude Code skills):

# Developer runs:
git diff main | claude code --skill pr-description

# Gets back in 30 seconds:
- Complete, properly filled template
- Type safety checklist pre-validated
- Performance implications flagged
- Specific test cases suggested
- Professional, consistent quality

# Developer adds:
- Screenshots (5 min)
- Business context (2 min)
- Review and submit (1 min)

# Total time: 8 minutes
# Quality: Consistently high

Getting Started with Skills

Week 1: Create basic skill

mkdir -p .claude/skills
# Copy pr-description.md template
# Test it on a sample PR

Week 2: Add checklist validator

# Create pr-checklist-validator.md
# Run on existing PRs to validate

Week 3: Create PR-type-specific skills

# Add pr-bug-fix.md
# Add pr-feature.md
# Add pr-refactor.md

Week 4: Team rollout

# Document in README
# Share with team
# Get feedback and iterate

Using AI Tools with These Templates

Don't fill out these templates manually. Use AI to automate the process while maintaining consistency.

Claude Code Skills: Automating Template Usage

Claude Code skills give you perfect repeatability. Create a skill once, use it on every PR.

Setting Up Template Skills

Step 1: Create .claude/skills/ directory

mkdir -p .claude/skills

Step 2: Create a skill for the Complete TypeScript PR Template

Create .claude/skills/typescript-pr-complete.md:

# Complete TypeScript PR Description Generator

You are helping a developer create a comprehensive TypeScript PR description.

## Your Task

Analyze the git diff and generate a complete PR description following the 
Complete TypeScript PR Template.

## Template Structure

Generate this exact structure:

## Summary
[1-2 sentence overview of changes]

## Problem
[What bug/feature? Why does it matter?]

**Impact:**
[How many users? Business context? Severity?]

## Solution
[High-level technical approach]

**Type Safety:**
[New interfaces? Breaking type changes? Type improvements?]

**Alternatives Considered:**
[What else was tried? Why this approach?]

## Type of Change
[Check ALL that apply]
- [ ] 🐛 Bug fix (non-breaking)
- [ ] ✨ New feature (non-breaking)
- [ ] 💥 Breaking change
- [ ] 📝 Documentation
- [ ] ♻️ Refactor
- [ ] ⚡ Performance
- [ ] ✅ Tests
- [ ] 🔒 Security fix
- [ ] ♿ Accessibility improvement

## TypeScript Checklist
Analyze the diff and check items based on what you find:
- [ ] No new `any` types (or justified with comments)
- [ ] No `@ts-ignore` without explanation
- [ ] All new interfaces/types are exported from appropriate files
- [ ] Type imports use `import type` syntax
- [ ] No type assertions without runtime validation
- [ ] Discriminated unions used for state machines
- [ ] Generic types have proper constraints
- [ ] Return types are explicit for public APIs

## Testing

**Manual Testing:**
[List what should be manually tested based on changes]
- [ ] Tested happy path
- [ ] Tested error cases
- [ ] Tested edge cases

**Automated Testing:**
[Suggest what tests should be added based on changes]
- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] All tests passing locally

**Test Cases:**
[List specific test scenarios based on code changes]

## Performance
[Check items that apply based on diff analysis]
- [ ] No performance regressions
- [ ] Expensive operations are memoized
- [ ] No unnecessary re-renders
- [ ] Bundle size impact checked
- [ ] Large imports are code-split

## Security
[Check items that apply based on diff analysis]
- [ ] No sensitive data in logs
- [ ] No hardcoded secrets
- [ ] User input is validated/sanitized
- [ ] No XSS vulnerabilities
- [ ] Authentication/authorization checked

## Accessibility
[Check items if UI changes detected]
- [ ] Semantic HTML used
- [ ] ARIA labels added where needed
- [ ] Keyboard navigation works
- [ ] Focus management is correct
- [ ] Color contrast is sufficient

## Breaking Changes
[List any breaking changes detected, or state "None"]

**Migration Guide:**
[If breaking changes, provide migration steps]

## Screenshots/Videos
[If UI changes detected, request: "REQUIRED: Add before/after screenshots"]

## Deployment Notes
[Check items based on changes detected]
- [ ] No database migrations required
- [ ] No environment variable changes required
- [ ] No infrastructure changes required

**Rollback Plan:**
[Suggest rollback approach based on change type]

## Related
Fixes #[FILL IN ISSUE NUMBER]

## Reviewer Notes
[Suggest what reviewers should focus on based on the changes]

---

## Instructions

1. **Analyze the diff carefully:**
   - Identify file changes, additions, deletions
   - Look for TypeScript-specific patterns
   - Detect type changes, new interfaces, breaking changes
   - Identify performance implications
   - Flag security concerns

2. **Be specific, not generic:**
   - Instead of "fixed bug", say "fixed null reference error in UserProfile"
   - Instead of "added tests", say "added unit tests for email validation edge cases"
   - Include actual type names, file names, line numbers where relevant

3. **Check checklists intelligently:**
   - ✅ Check items that apply based on actual code changes
   - ⬜ Leave unchecked if not applicable
   - Don't check everything blindly

4. **Identify gaps:**
   - If no tests found, flag: "⚠️ No tests detected. Recommend adding tests."
   - If new `any` types found, flag: "⚠️ New `any` types at lines X, Y, Z. Justify or replace."
   - If performance concerns, flag: "⚠️ Large import detected. Consider code-splitting."

5. **Use placeholders for unknown info:**
   - If you can't determine impact: "[NEEDS AUTHOR: User impact numbers]"
   - If screenshots needed but unavailable: "REQUIRED: Add screenshots of [specific UI change]"
   - If related issue unknown: "Fixes #[FILL IN]"

6. **Maintain professional tone:**
   - Clear and concise
   - Technical but readable
   - Constructive, not critical

## Output Format

Generate the complete template with ALL sections filled out based on diff analysis.
Use markdown formatting. Be thorough but concise.

Step 3: Use the skill

# Generate PR description from your changes
git diff main | claude code --skill typescript-pr-complete

# Or save to file
git diff main | claude code --skill typescript-pr-complete > pr-description.md

Step 4: Review and enhance

The skill fills out 80% of the template. You add:

Actual user impact numbers
Screenshots
Specific testing you performed
Business context

Time saved: 15 minutes → 5 minutes

Bug Fix Template Skill

Create .claude/skills/typescript-pr-bugfix.md:

# Bug Fix PR Generator

Generate a bug fix PR description using the Bug Fix Template.

## Template

## Bug Fix: [Brief Description]

## Bug Details

**What's Broken:**
[Analyze diff to identify the bug]

**Steps to Reproduce:**
[Infer from code or state: "NEEDS AUTHOR: Add reproduction steps"]
1. 
2. 
3. 

**Expected Behavior:**
[What should happen]

**Actual Behavior:**
[What actually happens - infer from code]

**Impact:**
- Severity: [Analyze code to estimate: Critical/High/Medium/Low]
- Affected Users: [NEEDS AUTHOR: Provide number or percentage]
- Frequency: [Infer: always/often/sometimes/rare]
- Support Tickets: [NEEDS AUTHOR: Provide count if available]

## Root Cause

[Analyze diff to explain why this bug exists]

## Solution

[Explain the fix based on code changes]

**Code Changes:**
[List files modified and what changed in each]

## Testing

**Reproduction:**
- [ ] Reproduced bug before fix
- [ ] Verified fix resolves issue
- [ ] Cannot reproduce bug after fix

**Regression Testing:**
- [ ] Related features still work
- [ ] Edge cases covered
- [ ] Added regression test

**Test Cases:**
[Based on code analysis, suggest specific test cases]

## Type Safety
[Check items based on diff]
- [ ] No new `any` types
- [ ] Null/undefined handling improved
- [ ] Type guards added where needed

## Risks

**Rollback Plan:**
[Suggest rollback strategy based on change scope]

**Monitoring:**
[Suggest what metrics to watch]

Fixes #[FILL IN]

## Instructions

1. Identify the bug from the diff
2. Explain root cause clearly
3. Describe the fix
4. Suggest comprehensive test cases
5. Flag if reproduction steps or impact metrics needed from author

Usage:

git diff main | claude code --skill typescript-pr-bugfix

Feature Template Skill

Create .claude/skills/typescript-pr-feature.md:

# Feature PR Generator

Generate a feature PR description using the New Feature Template.

## Template

## Feature: [Feature Name from code analysis]

## User Story

**As a** [Infer user type from feature]
**I want** [Infer goal from code]
**So that** [Infer benefit]

## Business Value

**Why This Feature:**
[Analyze code to understand purpose, or state: "NEEDS AUTHOR: Business justification"]

**Success Metrics:**
[NEEDS AUTHOR: Add metrics]
- Metric 1: 
- Metric 2: 

## Implementation

**User Flow:**
[Infer from UI components and logic]
1. 
2. 
3. 

**Technical Approach:**
[Describe architecture from code analysis]

**TypeScript Patterns Used:**
[Check what's actually in the code]
- [ ] Discriminated unions for state
- [ ] Generic components
- [ ] Type-safe API calls
- [ ] Proper error types

## API Changes

**New Endpoints:**
[List if backend changes detected]

**New Types:**

typescript
[Extract and list new interfaces/types from diff]


**Breaking Changes:**
[Flag any breaking changes in API or types]

## UI/UX Changes

**Screenshots:**
[If UI changes detected: "REQUIRED: Add before/after screenshots"]

**Responsive Design:**
- [ ] Mobile [needs testing]
- [ ] Tablet [needs testing]
- [ ] Desktop [needs testing]

**Accessibility:**
[Check based on code]
- [ ] Keyboard navigation works
- [ ] Screen reader compatible
- [ ] ARIA labels added

## Testing

**Feature Testing:**
[Suggest test cases based on feature logic]
- [ ] Happy path works
- [ ] Error handling works
- [ ] Edge cases handled

**Integration Testing:**
[Suggest integration tests needed]

**Performance:**
[Flag if performance testing needed]

## Documentation

- [ ] User documentation needed
- [ ] API documentation needed
- [ ] Code comments added
- [ ] README updated

## Rollout Plan

**Feature Flag:**
[Suggest feature flag name: `ENABLE_[FEATURE_NAME]`]

**Phased Rollout:**
- [ ] Internal testing
- [ ] Beta users
- [ ] Gradual rollout

Implements #[FILL IN]

Checklist Validator Skill

Create .claude/skills/typescript-pr-checklist.md:

# PR Checklist Validator

Validate PR changes against all TypeScript checklists.

## Your Task

Analyze the git diff and validate against EVERY checklist item.

## Checklists to Validate

### Type Safety Checklist

**Check each item:**
- No explicit `any` types (or justified)
- No `@ts-ignore` comments (or explained)
- Type imports use `import type`
- Null checks before access
- Type assertions justified
- Generic types have proper constraints
- Return types explicit for public APIs
- Discriminated unions for state machines

### Performance Checklist

**Check each item:**
- No unnecessary re-renders
- useMemo for expensive calculations
- useCallback for callbacks
- React.memo for expensive components
- Code splitting implemented
- No large library imports (tree-shaking)
- Images optimized

### Accessibility Checklist

**Check each item:**
- Semantic HTML used
- ARIA labels where needed
- Keyboard navigation works
- Focus management correct
- Color contrast sufficient
- Alt text for images
- Form labels associated

### Security Checklist

**Check each item:**
- No hardcoded secrets
- Input validated/sanitized
- No XSS vulnerabilities
- No sensitive data in logs
- Authentication/authorization checked
- Cookies are Secure and HttpOnly

## Output Format

For each checklist:

### [Checklist Name] - [✅ PASS | ⚠️ WARNINGS | ❌ FAIL]

**Status:**
- ✅ All checks pass
- ⚠️ [Count] warnings found
- ❌ [Count] critical issues found

**Issues:**

🔴 **Critical Issues (must fix before merge):**
- `File.tsx:45` - Hardcoded API key
- `Component.tsx:67` - XSS vulnerability

🟡 **Warnings (should fix):**
- `UserProfile.tsx:23` - Missing null check
- `Dashboard.tsx:89` - Expensive calculation not memoized

🟢 **Suggestions (nice to have):**
- `api.ts:12` - Could use `import type`

**Summary:**
[Overall assessment and priority recommendations]

## Instructions

1. Scan diff for each checklist category
2. Flag violations with file:line references
3. Provide specific fix suggestions
4. Prioritize: Critical → Warning → Suggestion
5. Be thorough but constructive

Usage:

# Validate your PR
git diff main | claude code --skill typescript-pr-checklist

# Save validation report
git diff main | claude code --skill typescript-pr-checklist > checklist-validation.md

GitHub Copilot Integration

Copilot works great for quick template filling in the GitHub UI.

In the GitHub PR description field:

Type # to trigger Copilot
Select "Generate PR description"
Copilot analyzes your diff and suggests description

Or use Copilot Chat in VS Code:

@workspace Generate a TypeScript PR description for my staged changes.

Use this template structure:
- Summary
- Problem (with business impact)
- Solution (with TypeScript considerations)
- Testing (manual + automated)
- TypeScript Checklist
- Performance impact
- Security review
- Screenshots (if UI changes)

Be specific and thorough.

Copilot strengths:

✅ Fast (built into IDE/GitHub)
✅ Understands workspace context
✅ Good for small PRs

Copilot limitations:

❌ Less customizable
❌ Can't enforce team-specific templates
❌ Inconsistent output
❌ Not version controlled

Best for: Quick bug fixes, small features under 200 lines

The Hybrid Workflow (Best Practice)

Combine skills + manual input + AI polish:

# Step 1: Generate base description (2 minutes)
git diff main | claude code --skill typescript-pr-complete > pr.md

# Step 2: Validate against checklists (1 minute)
git diff main | claude code --skill typescript-pr-checklist >> pr.md

# Step 3: Add your context (3 minutes)
# Edit pr.md to add:
# - Actual user impact numbers
# - Screenshots
# - Specific testing details
# - Business justification

# Step 4: Polish (1 minute)
# Paste pr.md into Claude:
# "Make this more concise and remove any redundancy"

# Step 5: Create PR (1 minute)
gh pr create --title "Fix: User profile crash on null email" --body-file pr.md

# Total time: 8 minutes (vs 20 minutes manual)

Complete Skills Library

Build a collection for your team:

.claude/skills/
├── typescript-pr-complete.md      # Full template
├── typescript-pr-bugfix.md        # Bug fix template
├── typescript-pr-feature.md       # Feature template
├── typescript-pr-refactor.md      # Refactoring template
├── typescript-pr-dependency.md    # Dependency update
├── typescript-pr-performance.md   # Performance optimization
├── typescript-pr-security.md      # Security fix
├── typescript-pr-checklist.md     # Validate all checklists
├── typescript-type-safety.md      # Deep type safety check
└── typescript-breaking-changes.md # Breaking change detector

Team README section:

## PR Descriptions

We use Claude Code skills for consistent, high-quality PR descriptions.

### Quick Start

bash

Complete PR description

git diff main | claude code --skill typescript-pr-complete

Validate checklists

git diff main | claude code --skill typescript-pr-checklist

Bug fix specific

git diff main | claude code --skill typescript-pr-bugfix


### Available Skills

- `typescript-pr-complete` - Full template with all sections
- `typescript-pr-bugfix` - Bug fix template
- `typescript-pr-feature` - New feature template
- `typescript-pr-refactor` - Refactoring template
- `typescript-pr-checklist` - Validate against all checklists
- `typescript-type-safety` - Deep type safety analysis

See `.claude/skills/` for details.

Automating with GitHub Actions

Run skills in CI to enforce standards:

# .github/workflows/pr-validation.yml
name: PR Quality Check

on:
  pull_request:
    types: [opened, synchronize]

jobs:
  validate:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
        with:
          fetch-depth: 0

      - name: Setup Node
        uses: actions/setup-node@v3

      - name: Install Claude Code
        run: npm install -g @anthropic-ai/claude-code
        env:
          ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}

      - name: Generate diff
        run: |
          git diff origin/${{ github.base_ref }}...HEAD > /tmp/pr.diff

      - name: Validate checklist
        id: validate
        run: |
          claude code --skill typescript-pr-checklist \
            --input /tmp/pr.diff > /tmp/validation.md

      - name: Check for critical issues
        run: |
          if grep -q "🔴.*Critical" /tmp/validation.md; then
            echo "Critical issues found!"
            cat /tmp/validation.md
            exit 1
          fi

      - name: Post validation as comment
        if: always()
        uses: actions/github-script@v6
        with:
          script: |
            const fs = require('fs');
            const validation = fs.readFileSync('/tmp/validation.md', 'utf8');

            github.rest.issues.createComment({
              issue_number: context.issue.number,
              owner: context.repo.owner,
              repo: context.repo.repo,
              body: `## 🤖 Automated PR Validation\n\n${validation}`
            });

Result: Every PR gets automatically validated against all checklists.

Real-World Example

Before AI tools:

Developer writes PR manually:
- Fills out template: 15 minutes
- Forgets to check type safety items
- Misses performance implications
- Generic testing description
- Inconsistent with other PRs

Reviewer asks questions:
- What's the impact?
- Why this approach?
- Where are the tests?

Back and forth: 2-3 days

After AI tools (Claude Code skills):

# Developer runs (30 seconds):
git diff main | claude code --skill typescript-pr-complete > pr.md
git diff main | claude code --skill typescript-pr-checklist >> pr.md

# Reviews generated description (2 minutes)
# Adds screenshots, numbers, context (3 minutes)
# Creates PR (1 minute)

Total: 7 minutes
Quality: Consistently high
All checklist items validated
No back-and-forth needed

Reviewer approves: Same day

Comparison: Manual vs AI-Assisted

Aspect	Manual Template	With Skills
Time to create	15-20 min	5-8 min
Consistency	Varies by person	Identical every time
Completeness	Often missing items	All sections filled
Quality	Depends on experience	Consistently high
Type safety check	Manual (often skipped)	Automatic
Checklist validation	Manual (often incomplete)	Automatic
Team onboarding	Slow (learn template)	Fast (run command)
Version control	Template only	Template + automation

ROI of setting up skills:

Setup time: 2 hours (create skills)
Time saved per PR: 10-15 minutes
Break-even: ~10 PRs
Annual savings: ~50 hours per developer

Getting Started This Week

Monday: Create basic skill

mkdir -p .claude/skills
# Copy typescript-pr-complete.md skill template
# Test on a sample PR

Tuesday: Test and refine

# Run on 3 different PR types
# Adjust skill based on output quality

Wednesday: Add checklist validator

# Create typescript-pr-checklist.md
# Test validation on existing PRs

Thursday: Document for team

# Add section to README
# Create quick reference guide

Friday: Team rollout

# Demo to team
# Get feedback
# Iterate based on usage

Next week: 100% adoption, consistently great PRs.

Automation Ideas

Pre-commit Checklist

Use Husky to enforce checklist:

// package.json
{
  "husky": {
    "hooks": {
      "pre-commit": "npm run checklist"
    }
  },
  "scripts": {
    "checklist": "npm run lint && npm run type-check && npm test"
  }
}

Danger.js for PR Validation

// dangerfile.js
import { danger, warn, fail } from 'danger';

// Check PR size
const bigPR = danger.github.pr.additions + danger.github.pr.deletions > 500;
if (bigPR) {
  warn('This PR is quite large. Consider splitting it.');
}

// Require tests for code changes
const hasCodeChanges = danger.git.modified_files.some(f => 
  f.endsWith('.ts') || f.endsWith('.tsx')
);
const hasTestChanges = danger.git.modified_files.some(f => 
  f.includes('.test.')
);

if (hasCodeChanges && !hasTestChanges) {
  warn('No tests added. Consider adding tests for your changes.');
}

// Check for console.log
const consoleLogFound = danger.git.modified_files.some(file => {
  const content = danger.git.fileMatch(file).includes('console.log');
  return content;
});

if (consoleLogFound) {
  fail('console.log found. Remove debug code before merging.');
}

GitHub Actions Checklist

# .github/workflows/pr-checklist.yml
name: PR Checklist

on: [pull_request]

jobs:
  checklist:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3

      - name: Check for console.log
        run: |
          if grep -r "console.log" src/; then
            echo "console.log found in source code"
            exit 1
          fi

      - name: Check for TODO comments
        run: |
          if grep -r "TODO" src/; then
            echo "TODO comments found. Create issues instead."
            exit 1
          fi

      - name: Check bundle size
        run: npm run build && npm run size-check

Customizing for Your Team

Start Simple

Begin with the basic template, then add sections your team needs.

Team-Specific Sections

For e-commerce:

## Business Impact
- [ ] Revenue impact estimated
- [ ] Conversion tracking added
- [ ] Analytics events implemented

For healthcare:

## Compliance
- [ ] HIPAA compliance verified
- [ ] PHI handling reviewed
- [ ] Audit logging implemented

For B2B SaaS:

## Customer Impact
- [ ] Customer-facing changes documented
- [ ] Support team notified
- [ ] Release notes drafted

Progressive Enhancement

Week 1: Basic template
Week 2: Add type safety checklist
Week 3: Add performance checklist
Week 4: Add accessibility checklist
Week 5: Refine based on feedback

Conclusion

Great PRs aren't accidental. They follow templates and checklists.

Templates ensure:

✅ Consistent structure
✅ Complete information
✅ Faster reviews
✅ Better documentation

Checklists prevent:

❌ Type safety issues
❌ Performance regressions
❌ Security vulnerabilities
❌ Accessibility gaps

Copy these templates. Use these checklists. Ship better code.

Start with one template. Customize it for your team. Add checklists as needed. Automate what you can.

Your reviewers will thank you. Your users will thank you. Your future self will thank you.

Which template will you implement first? Share your customizations!

Writing PR Descriptions That Get Approved Faster

Tarun Moorjani — Tue, 03 Mar 2026 16:59:25 +0000

I've reviewed thousands of PRs. Some get approved in 10 minutes. Others sit for days.

The difference isn't code quality. It's the PR description.

Bad PR descriptions look like this:

Title: Fix bug

Description: Fixed the thing

I have to:

Clone the PR
Read every line of code
Figure out what changed
Guess why it changed
Hunt for the related issue
Ask questions in comments
Wait for answers
Re-review

Time to approve: 2-3 days, multiple review cycles

Good PR descriptions look like this:

Title: Fix user profile crash when email is null

## Problem
Users with null emails (legacy data from 2019 migration) crash the profile 
page when clicking "Edit Profile". Error: "Cannot read property 'toLowerCase' 
of null"

Affects ~500 users based on database query.

## Solution
Added null check before calling toLowerCase(). If email is null, show 
"Add email" prompt instead.

## Testing
- [x] Manual: Tested with user ID 12345 (has null email)
- [x] Unit: Added test for null email case
- [x] Verified existing tests still pass

## Screenshots
[Before] - Crash screen
[After] - "Add email" prompt

Fixes #1234

I can:

Understand the problem immediately
See the solution makes sense
Trust it's been tested
Approve in 5 minutes

Time to approve: 5-10 minutes, one review cycle

Let me show you how to write PR descriptions that get approved fast.

The 5-Minute Rule

If a reviewer can't understand your PR in 5 minutes, you've failed.

Your goal: Make reviewing effortless.

Every second a reviewer spends confused is a second they might:

Ask questions (delays approval)
Request changes (delays approval)
Ignore your PR (delays approval)

Your job: Answer all their questions before they ask.

The PR Description Template

Copy this. Customize it. Use it every time.

## Problem

[What bug are you fixing? What feature are you adding? Why does this matter?]

[Impact: How many users? How severe? Business context?]

## Solution

[High-level approach. NOT line-by-line code explanation.]

[Key decisions made and why]

[Any alternatives considered and why rejected]

## Testing

- [ ] Manual testing steps performed
- [ ] Unit tests added/updated
- [ ] Integration tests added/updated
- [ ] Tested edge cases: [list them]

## Risks

[Any concerns? Known limitations? Future work needed?]

## Screenshots/Videos

[If UI change: before/after screenshots or screen recording]

## Related

Fixes #[issue number]
Related to #[other PR or issue]
Depends on #[blocking PR]

---

## Reviewer Notes

[Anything specific reviewers should focus on? Anything you're unsure about?]

Section-by-Section Breakdown

Title: Make It Searchable

Bad titles:

Fix bug
Update component
Changes
WIP: stuff

Good titles:

Fix profile crash when user email is null
Add pagination to user management table
Upgrade React Router from v5 to v6
Refactor authentication to use OAuth2

Template:

[Action] [What] [Where/When/Why]

Examples:

Fix infinite loop in useEffect when deps change rapidly
Add loading state to checkout form submit button
Remove deprecated props from Button component
Upgrade TypeScript from 4.9 to 5.3

Why it matters: Reviewers often search for PRs. "Fix bug" is unsearchable. "Fix profile crash when email is null" is specific.

Problem: Why Does This PR Exist?

This is the most important section. If reviewers don't understand the problem, they can't evaluate the solution.

Bad:

## Problem
Need to fix the profile page

Good:

## Problem

Users with null emails (legacy data from 2019 migration) crash when clicking 
"Edit Profile" button. 

Error: `Cannot read property 'toLowerCase' of null` at ProfileForm.tsx:45

**Impact:**
- Affects ~500 users (0.5% of active users)
- Support tickets: 12 in last week
- Critical path: Users can't update their profiles
- Business impact: Users can't change their notification preferences

**Root cause:**
The 2019 migration from old auth system didn't require emails. Current code 
assumes all users have emails.

What to include:

Exact error message (if it's a bug)
Impact: how many users, how severe
Business context: why this matters
Root cause: what's actually wrong

Bad example - vague:

## Problem
The form is broken

Good example - specific:

## Problem

Checkout form doesn't validate credit card expiration dates, allowing users to 
submit expired cards.

**Impact:**
- Payment processor rejects 15% of transactions
- Users see generic "Payment failed" error
- Support receives ~50 tickets/day asking why payment failed
- Lost revenue: ~$50k/month in abandoned carts

**Root cause:**
Validation regex only checks format (MM/YY) but doesn't compare to current date.

Solution: What Did You Do?

Explain your approach at a high level. Don't describe every line of code.

Bad:

## Solution
Added a check

Bad (too detailed):

## Solution
On line 45, I added an if statement that checks if email is null. If it is, 
I set a variable called showAddEmail to true. Then on line 67, I check if 
showAddEmail is true, and if so, I render a div with className "add-email-prompt" 
that contains a button...

Good:

## Solution

Added null check before processing email:
1. Check if user.email is null
2. If null, show "Add email" prompt instead of edit form
3. Prompt includes link to account settings to add email

**Key decision:** 
Chose to show prompt rather than pre-populate with empty string, because empty 
string would allow users to save profile without fixing the root issue.

**Alternative considered:** 
Backfill all null emails with "noemail@example.com". Rejected because this 
creates invalid data and doesn't solve the problem for future users.

What to include:

High-level approach (numbered steps work well)
Key decisions and reasoning
Alternatives considered (shows you thought it through)

Pro tip: If you considered multiple approaches, show them:

## Solution

**Approach taken:** Add validation to frontend form

**Why not these alternatives:**
1. ❌ Fix in backend - Requires API change, affects mobile apps
2. ❌ Backfill null emails - Creates invalid data
3. ❌ Disable form for null emails - Poor UX, users can't update anything

Testing: How Do I Know It Works?

Show you've tested thoroughly. Reviewers trust tested code.

Bad:

## Testing
Tested it, works fine

Good:

## Testing

**Manual testing:**
- [x] Tested with user ID 12345 (has null email) - shows prompt correctly
- [x] Tested with user ID 67890 (has email) - edit form works normally
- [x] Clicked "Add email" link - navigates to settings correctly
- [x] Added email in settings, returned to profile - edit form now appears

**Automated tests:**
- [x] Added unit test: `ProfileForm.test.tsx` - null email case
- [x] Added unit test: `ProfileForm.test.tsx` - valid email case
- [x] Updated integration test: `profile-flow.test.tsx`
- [x] All existing tests passing (142/142)

**Edge cases tested:**
- [x] Email is empty string (not just null)
- [x] Email is whitespace only
- [x] User spams "Add email" button (no duplicate navigations)

**Browser testing:**
- [x] Chrome 120
- [x] Safari 17
- [x] Firefox 121
- [x] Mobile Safari iOS 17

Checklist template:

## Testing

**Functionality:**
- [ ] Happy path works
- [ ] Error cases handled
- [ ] Edge cases tested

**Tests added:**
- [ ] Unit tests for new logic
- [ ] Integration tests for user flow
- [ ] All existing tests still pass

**Manual verification:**
- [ ] Tested locally
- [ ] Tested in staging
- [ ] Verified fix with reproduction steps from bug report

Risks: What Could Go Wrong?

Be honest about risks. Reviewers appreciate transparency.

Bad:

## Risks
None

Good:

## Risks

**Known limitations:**
- This fixes the immediate crash, but doesn't solve the root problem of users 
  having null emails. Follow-up PR needed to prompt users to add emails.

**Performance impact:**
- Added database query to check email status. Query is fast (~5ms) and cached, 
  but monitoring recommended.

**Rollback plan:**
- If issues arise, can disable via feature flag `PROFILE_NULL_EMAIL_CHECK`
- No database migrations, safe to rollback

**Future work needed:**
- #1235 - Backfill null emails (non-urgent)
- #1236 - Add email requirement to onboarding flow

What to include:

Known limitations
Performance implications
Rollback strategy
Future work needed

Screenshots/Videos: Show, Don't Tell

For UI changes, screenshots are mandatory.

Template:

## Screenshots

**Before:**
![Before](url-to-image)
[Description of problem]

**After:**
![After](url-to-image)
[Description of fix]

**Screen recording:**
[Link to Loom/video showing interaction]

Pro tips:

Use side-by-side before/after when possible
Annotate screenshots (arrows, circles) to highlight changes
For interactions, use screen recordings (Loom, CloudApp)
For responsive changes, show mobile + desktop

Example:

## Screenshots

**Before:** Empty email crashes the page
![crash](https://i.imgur.com/crash.png)

**After:** Shows "Add email" prompt instead
![fixed](https://i.imgur.com/fixed.png)

**Mobile view:**
![mobile](https://i.imgur.com/mobile.png)

**Demo video:** https://www.loom.com/share/abc123
Shows full flow: crash → fix → adding email → form appearing

Related: Link Everything

Connect your PR to the broader context.

Template:

## Related

Fixes #1234
Closes #1235
Resolves #1236

Part of #1240 (epic)
Depends on #1238 (must merge first)
Blocks #1242 (waiting on this)
Related to #1237 (similar issue)

[Design spec](link-to-figma)
[Tech spec](link-to-doc)
[Slack discussion](link-to-thread)

GitHub keywords that auto-close issues:

Fixes #123
Closes #123
Resolves #123

Why link everything:

Reviewers see full context
Issues auto-close when PR merges
Future developers understand the history
Stakeholders can track progress

Reviewer Notes: Direct Attention

Tell reviewers what to focus on and what to ignore.

Template:

## Reviewer Notes

**Focus areas:**
- `ProfileForm.tsx` - Main logic change
- `profile.test.tsx` - New test coverage

**Can skip:**
- `package-lock.json` - Dependency update (unrelated)
- `translations/` - Auto-generated from i18n tool

**Questions for reviewers:**
- Line 45: Is this error message clear enough?
- Line 67: Should we log this to analytics?

**Known issues (will fix before merge):**
- [ ] TODO on line 123 - will implement before merging
- [ ] Commented-out code - will remove

Why this helps:

Saves reviewers time
Gets feedback on specific concerns
Shows you're thoughtful about the review

Size Matters: Keep PRs Small

Bad: 47 files changed, 3,000 lines added

Reviewers see this and:

Put it off until "later"
Skim instead of reviewing carefully
Miss bugs

Good: 3 files changed, 150 lines added

Reviewers can:

Review in one sitting
Understand all changes
Provide thoughtful feedback

Guidelines:

Ideal: < 200 lines changed
OK: 200-400 lines changed
Large: 400-800 lines changed (split if possible)
Too large: > 800 lines changed (definitely split)

How to split large PRs:

Epic: Implement user profile redesign

PR 1: Add database schema for profile fields (#1234)
PR 2: Create ProfileCard component (#1235) ← You are here
PR 3: Integrate ProfileCard into profile page (#1236)
PR 4: Add profile editing flow (#1237)
PR 5: Migrate existing users to new schema (#1238)

Each PR:

Is independently reviewable
Can be merged separately
Builds on previous PRs

In your PR description: ""

## Context

This is part 2 of 5 in the profile redesign epic (#1233).

**Previous PRs:**
- [x] #1234 - Database schema (merged)

**This PR:**
- Creates reusable ProfileCard component
- Adds unit tests
- Adds Storybook stories

**Next PRs:**
- [ ] #1236 - Integration into profile page
- [ ] #1237 - Editing flow
- [ ] #1238 - User migration

Real Examples: Bad vs. Good

Example 1: Bug Fix

❌ Bad PR:

Title: fix

fixed the bug where the thing broke

Reviewer questions:

What bug?
What thing?
How did it break?
How did you fix it?
How do I test it?
Is this related to any issue?

✅ Good PR:

Title: Fix duplicate charges when user double-clicks "Pay Now"

## Problem

Users who double-click the "Pay Now" button get charged twice. 

**Impact:**
- 23 duplicate charges in the last week
- $4,500 in refunds processed
- 15 angry support tickets
- Payment processor flagging us for suspicious activity

**Root cause:**
No debouncing on submit button. Each click triggers a separate API call.

## Solution

1. Disable button immediately on first click
2. Re-enable only after API response (success or error)
3. Add 1-second debounce as secondary protection

**Why not just debounce:**
Debouncing alone isn't enough. If API is slow, users might click again after 
1 second. Disabling the button is more reliable.

## Testing

**Manual:**
- [x] Single click - works normally
- [x] Double click - only one charge created
- [x] Spam clicking - only one charge created
- [x] Slow network (throttled to 3G) - still works correctly

**Automated:**
- [x] Added test: double-click prevention
- [x] Added test: button re-enables on error
- [x] All existing payment tests passing (24/24)

**Verified in production:**
- [x] Tested in staging with production-like load
- [x] Checked Datadog for error rate (0% increase)

## Risks

**Low risk change:**
- Only affects button state, no payment logic changes
- Can rollback via feature flag `PAYMENT_BUTTON_DEBOUNCE`
- If button gets stuck disabled, users can refresh page

## Screenshots

**Before:** Button stays enabled, multiple clicks allowed
![before](https://i.imgur.com/before.png)

**After:** Button disables on first click, re-enables after response
![after](https://i.imgur.com/after.png)

**Demo:** https://www.loom.com/share/abc123
Shows spam clicking, only one charge created

Fixes #1234

Example 2: New Feature

❌ Bad PR:

Title: Add pagination

Added pagination to the table

✅ Good PR:

Title: Add pagination to user management table (1000+ users)

## Problem

User management page loads all users at once, causing:
- 15+ second load time for admins with 1000+ users
- Browser memory issues (crashes on 2000+ users)
- Unable to find specific users quickly

**User feedback:**
"I have 1500 users and the page won't even load anymore" - Customer #4521

## Solution

Implement server-side pagination:
1. Load 50 users per page (configurable)
2. Add pagination controls (prev/next, page numbers)
3. Maintain filters and search across pages
4. Show total count and current range

**Technical approach:**
- Backend: Added `limit` and `offset` query params to `/api/users`
- Frontend: useQuery with pagination state
- Maintained backward compatibility (defaults to 50)

**Design decisions:**
- 50 users per page: balances load time vs. scrolling
- Show 5 page numbers at a time (... for rest)
- Preserve search/filters when changing pages

## Testing

**Manual:**
- [x] Tested with 10 users (shows 1 page)
- [x] Tested with 150 users (shows 3 pages)
- [x] Tested with 1500 users (page load < 2 seconds)
- [x] Search works across all pages
- [x] Filters work with pagination
- [x] URL updates with page number (shareable links)
- [x] Back button works correctly

**Automated:**
- [x] Unit tests: pagination logic
- [x] Unit tests: page number calculation
- [x] Integration test: navigating between pages
- [x] Integration test: filters + pagination
- [x] API tests: limit/offset params

**Performance:**
- Before: 15s load (1000 users)
- After: 1.8s load (50 users per page)
- Backend query time: 45ms (with indexes)

## Risks

**API changes:**
- Added optional query params (backward compatible)
- Old API calls (no params) still work, default to 50

**Future improvements needed:**
- #1245 - Add "items per page" selector
- #1246 - Infinite scroll option
- #1247 - Better mobile pagination UI

## Screenshots

**Before:** All 1000+ users in one long list
![before](https://i.imgur.com/before.png)

**After:** Clean pagination, 50 users per page
![after](https://i.imgur.com/after.png)

**Pagination controls:**
![controls](https://i.imgur.com/controls.png)

**Mobile view:**
![mobile](https://i.imgur.com/mobile.png)

## Related

Fixes #1234
Part of performance epic #1200
Design: https://www.figma.com/file/abc123

Special Cases

Refactoring PRs

Title: Refactor authentication context to use zustand

## Why Refactor?

Current Context API implementation has issues:
- Causes unnecessary re-renders (entire app re-renders on auth change)
- Difficult to test (requires Provider wrapper)
- 200+ lines of boilerplate

## Changes

Replace Context API with zustand:
- ~200 lines removed
- Simpler API (`useAuth()` instead of `useContext(AuthContext)`)
- Better performance (selective subscriptions)
- Easier to test (no Provider needed)

## Migration Strategy

**Backward compatible:**
- Old Context API still works (deprecated)
- New zustand store available alongside
- Will remove Context in next PR after migration complete

**Migration path for other developers:**

diff

const { user } = useContext(AuthContext);
const user = useAuth(state => state.user);


## Testing

- [x] All existing tests still pass (no changes needed)
- [x] Added zustand store tests
- [x] Verified auth flow works identically
- [x] No console errors in dev/prod

## Performance

**Before:**
- Login triggers 47 component re-renders
- DevTools shows deep re-render tree

**After:**
- Login triggers 3 component re-renders
- Only components using auth state re-render

Fixes #1234 (performance issue)

Dependency Updates

Title: Upgrade React Router from v5 to v6

## Why Upgrade?

- v5 is deprecated (no security updates)
- v6 has better TypeScript support
- v6 has better hooks API
- Blocking upgrade to React 18

## Breaking Changes

**1. Route syntax changed:**

diff

} />


**2. useHistory → useNavigate:**

diff

const history = useHistory();
history.push('/home');
const navigate = useNavigate();
navigate('/home');


**3. Switch → Routes:**

diff

} />


## Migration Approach

**Completed:**
- [x] Updated all route definitions (34 files)
- [x] Replaced useHistory with useNavigate (67 locations)
- [x] Updated all tests (45 test files)
- [x] Updated type definitions

**Tested:**
- [x] All navigation flows work
- [x] URL params work
- [x] Nested routes work
- [x] Redirects work
- [x] 404 handling works

## Risks

**Large change:** 67 files modified
**Mitigation:** 
- Incremental testing (tested each route individually)
- Feature flag: `ENABLE_REACT_ROUTER_V6` (can rollback)
- Deployed to staging for 2 days (no issues)

## Related

Blocks #1240 (React 18 upgrade)
Migration guide: https://reactrouter.com/docs/en/v6/upgrading/v5

Templates for Different PR Types

Bug Fix Template

Title: [Fix: Brief description of bug]

## Problem
[What's broken? Error message? Impact?]

## Root Cause
[Why did this happen?]

## Solution
[What did you change?]

## Testing
- [ ] Reproduced bug before fix
- [ ] Verified fix resolves issue
- [ ] Tested edge cases
- [ ] Added regression test

## Risks
[Any concerns?]

Fixes #[issue]

Feature Template

Title: [Add: Brief description of feature]

## Why This Feature?
[User need? Business value? Use case?]

## Implementation
[Technical approach]

## Testing
- [ ] Manual testing completed
- [ ] Unit tests added
- [ ] Integration tests added
- [ ] Edge cases covered

## Screenshots
[Before/after if applicable]

## Related
Fixes #[issue]
[Design link]

Refactoring Template

Title: [Refactor: What you're refactoring]

## Why Refactor?
[Technical debt? Performance? Maintainability?]

## Changes
[What's different?]

## Migration Path
[How do other devs adapt?]

## Testing
- [ ] All existing tests pass
- [ ] No behavior changes
- [ ] Performance verified

Related to #[issue]

The Self-Review Checklist

Before submitting your PR, review it yourself:

**Before submitting:**

Title:
- [ ] Descriptive and searchable
- [ ] Follows format: [Action] [What] [Where/Why]

Description:
- [ ] Problem is clear
- [ ] Solution makes sense
- [ ] Testing is thorough
- [ ] Risks are addressed
- [ ] Screenshots included (if UI change)
- [ ] Links to related issues/docs

Code:
- [ ] Removed console.logs
- [ ] Removed commented-out code
- [ ] Removed TODO comments (or created issues)
- [ ] No merge conflicts
- [ ] CI passing

Polish:
- [ ] Spell-checked description
- [ ] Code is formatted (Prettier)
- [ ] Linted (ESLint)

Common Mistakes to Avoid

❌ Mistake 1: WIP PRs Without Context

Title: WIP

Description: work in progress, don't review yet

Better: Still provide context even if WIP

Title: [WIP] Add pagination to user table

## Goal
Implementing server-side pagination for tables with 1000+ rows

## Status
- [x] Backend API changes
- [x] Frontend pagination logic
- [ ] UI polish
- [ ] Tests

**Not ready for full review yet**, but happy to get feedback on approach.

**Specifically:**
- Does the API design make sense? (see `UserController.ts`)
- Is the pagination hook reusable enough? (see `usePagination.ts`)

❌ Mistake 2: Giant Unexplained Diffs

Title: Updates

47 files changed, 3,847 insertions(+), 2,103 deletions(-)

Description: Made some changes

Better: Explain large changes

Title: Upgrade to TypeScript 5.0 and fix type errors

## Changes

**67 files changed** - most changes are type fixes:

**Breaking type changes:**
- `strict: true` in tsconfig (caught 45 real bugs)
- `noUncheckedIndexedAccess: true` (array access now properly typed)

**Categories of changes:**
1. Added null checks (23 files)
2. Fixed 'any' types (15 files)  
3. Updated library type imports (18 files)
4. Misc type narrowing (11 files)

**Key files:**
- `tsconfig.json` - stricter settings
- `src/types/` - updated type definitions
- `src/utils/` - added type guards

**No behavior changes** - only type safety improvements

❌ Mistake 3: No Screenshots for UI Changes

Visual changes without visuals = reviewers have to check out your branch.

Always include:

Before/after screenshots
Different screen sizes (mobile, tablet, desktop)
Different states (loading, error, success)
Screen recording for interactions

❌ Mistake 4: Assuming Reviewers Have Context

You've been thinking about this problem for hours. Reviewers have 30 seconds.

Bad:

Fixed the race condition

Good:

Fixed race condition in useEffect when userId changes rapidly

## Problem
When user navigates quickly between profiles (rapid userId changes), 
multiple API calls fire simultaneously. The responses arrive out of order, 
causing wrong user's data to display.

Example: Click User A → User B → User C rapidly
Response order: C → A → B
Result: User B's data shows (wrong!)

Making Reviewers' Lives Easier

1. Add Comments to Complex Code

// In the PR description: ""
## Reviewer Notes

Line 45-67: Complex memoization logic
The triple-nested useMemo is intentional. We need to:
1. Memoize the filter function (inner)
2. Memoize the filtered data (middle)  
3. Memoize the sorted result (outer)

Tried simpler approaches but they caused performance issues with large datasets.

// In the code:
// NOTE: This triple-nesting is intentional for performance.
// Each level prevents re-computation when only specific deps change.
// See PR description for alternatives considered.
const sorted = useMemo(() => {
  const filtered = useMemo(() => {
    const filterFn = useMemo(() => {
      // ...

2. Explain Non-Obvious Decisions

## Design Decisions

**Why setTimeout instead of debounce?**
Debounce would delay the API call, but we want immediate feedback. setTimeout 
ensures button re-enables even if API call fails.

**Why not use a library for this?**
Evaluated `react-use` and `lodash`, but both added 20KB for a 5-line solution. 
Not worth the bundle size for this simple use case.

3. Point Out Potential Concerns

## Reviewer Notes

**Line 123:** You might think this could cause a memory leak, but it's fine 
because we clean up in the useEffect return function (line 145).

**Performance:** This does add an extra render, but only in the error case, 
which is rare. Happy path performance is unchanged.

The Ultimate PR Description

Here's a complete example that does everything right:

Title: Fix race condition in user profile when navigating rapidly

## Problem

**What's broken:**
When users navigate rapidly between profiles (clicking different users in quick 
succession), the wrong user's data displays.

**Why it happens:**
Multiple API calls fire simultaneously, but responses arrive out of order due 
to varying response times. The last response "wins" even if it's for an older 
navigation.

**Example scenario:**
1. User clicks Profile A (slow server, 2s response)
2. User clicks Profile B (fast server, 0.5s response)
3. Profile B loads (correct)
4. Profile A loads and overwrites Profile B (wrong!)

**Impact:**
- Affects power users who navigate quickly (~5% of users)
- Reported by 8 users in last 2 weeks
- Privacy concern: Users might see wrong person's data
- No data corruption (read-only), but poor UX and trust issue

**Root cause:**
useEffect doesn't cancel previous fetch when userId changes.

## Solution

Implemented AbortController to cancel in-flight requests:

1. Create AbortController in useEffect
2. Pass signal to fetch
3. Cancel previous request on cleanup
4. Ignore responses from cancelled requests

**Why AbortController:**
- Native browser API (no library needed)
- Proper way to cancel fetch requests
- Works across all modern browsers

**Alternatives considered:**
1. ❌ Track request IDs - complex, error-prone
2. ❌ Debounce navigation - poor UX, feels sluggish  
3. ✅ AbortController - clean, standard approach

**Code changes:**
- Modified `useUserProfile.ts` hook
- Added cleanup function to useEffect
- Added error handling for abort errors (expected, not logged)

## Testing

**Manual testing:**
- [x] Rapid navigation (10 users in 5 seconds) - correct user always shows
- [x] Slow network (throttled to 3G) - race condition fixed
- [x] Fast navigation then back button - works correctly
- [x] Navigation during loading - cancels and loads new user

**Automated tests:**
- [x] Unit test: AbortController cleanup
- [x] Unit test: Ignore aborted responses
- [x] Integration test: Rapid navigation scenario
- [x] All existing tests pass (87/87)

**Edge cases:**
- [x] Navigate away while loading - request cancelled
- [x] Navigate to same user - no unnecessary requests
- [x] Network error during navigation - error state shows correctly

**Browser testing:**
- [x] Chrome 120 ✓
- [x] Safari 17 ✓
- [x] Firefox 121 ✓
- [x] Mobile Safari iOS 17 ✓

## Performance Impact

**Before:** 
- 3 simultaneous requests for rapid navigation
- Wasted bandwidth
- Wrong data displayed

**After:**
- Only 1 active request at a time
- Previous requests cancelled
- Correct data always displayed

**Metrics:**
- No performance regression
- Actually reduces network traffic (fewer parallel requests)
- Memory usage unchanged

## Risks

**Low risk:**
- Small, focused change (1 file, 10 lines)
- Well-tested pattern (used in production by Facebook, Google)
- No database changes
- No API changes

**Browser support:**
- AbortController supported in all modern browsers
- IE11: Not supported, but we don't support IE11 anymore

**Rollback plan:**
- Feature flag: `USE_ABORT_CONTROLLER` (default: true)
- Can disable if issues arise
- No data migration needed

**Monitoring:**
- Added logging for abort errors (should be 0 in production)
- Dashboard: "Profile Load Success Rate" (should remain 99.8%)

## Screenshots

**Before:** Wrong user data shows after rapid navigation
![before](https://i.imgur.com/before.png)

**After:** Correct user always shows, old requests cancelled
![after](https://i.imgur.com/after.png)

**DevTools Network tab:**
Shows requests being cancelled (status: "cancelled")
![network](https://i.imgur.com/network.png)

**Demo video:** https://www.loom.com/share/abc123
Shows rapid navigation - correct user always displays

## Related

Fixes #1234
Related to #1200 (performance epic)
Similar fix: #1180 (same pattern for posts)

**Documentation updated:**
- Added AbortController pattern to wiki
- Updated `CONTRIBUTING.md` with example

## Reviewer Notes

**Focus areas:**
- `useUserProfile.ts` lines 23-45 - Main change
- `useUserProfile.test.tsx` - New test coverage

**Questions:**
1. Line 34: Should we retry on abort? I chose no, but open to discussion.
2. Line 42: Error logging - is this the right level (debug vs info)?

**Implementation details:**
- The cleanup function (line 45) runs on both unmount AND before next effect
- This ensures we cancel even if userId changes multiple times quickly
- AbortError is expected and not a real error, so we filter it in error logging

**Alternative approaches considered:**
See "Solution" section above. Happy to discuss if you prefer a different approach.

Using AI to Generate PR Descriptions

AI tools like Claude Code, GitHub Copilot, and Claude can dramatically speed up PR description writing. But use them wisely—AI should assist, not replace your thinking.

The AI Workflow

Step 1: Generate the diff context

# Get your changes
git diff main...your-branch > changes.diff

# Or for staged changes
git diff --cached > changes.diff

Step 2: Prompt the AI

Here's my go-to prompt for Claude:

I'm creating a PR. Here's my git diff:

[paste diff]

Please write a PR description following this template:

## Problem
[What bug/need does this address? Impact?]

## Solution  
[High-level approach, key decisions]

## Testing
[What testing was done?]

## Risks
[Any concerns or limitations?]

Generate a draft PR description. Focus on the "why" and "what", not 
line-by-line code explanation.

Step 3: Review and refine

AI gives you a solid first draft. Now add:

Business context AI doesn't know
Testing you actually performed
Screenshots (AI can't generate these)
Links to related issues
Specific reviewer questions

What AI Is Great At

✅ Summarizing code changes

// You changed 15 files
// AI reads the diff and says:

"This PR refactors authentication to use OAuth2 instead of session cookies.
Changes include:
- Replaced SessionAuth with OAuth2Provider
- Updated login flow to redirect to auth server
- Added token refresh logic
- Migrated existing users to new auth system"

✅ Identifying patterns

AI notices: "This change adds null checks in 8 files - looks like you're 
fixing a null reference issue throughout the codebase."

✅ Suggesting test cases

AI suggests: "Based on the null checks added, you should test:
- User with null email
- User with empty string email
- User with whitespace-only email"

✅ Writing first draft quickly
Instead of 10 minutes writing from scratch, AI gives you a 2-minute starting point.

What AI Needs Your Help With

❌ Business context
AI doesn't know:

How many users this affects
Why this matters to the business
Customer complaints that triggered this
Revenue impact

You add:

## Problem

[AI wrote: "Users with null emails can't access their profiles"]

Users with null emails (legacy data from 2019 migration) crash when clicking 
"Edit Profile". 

**Impact:** ← YOU ADD THIS
- Affects ~500 users (0.5% of active users)
- Support tickets: 12 in last week  
- Blocking users from updating notification preferences

❌ Why you chose this approach
AI can see what you did, but not why you rejected alternatives.

You add:

## Solution

[AI wrote: "Added null check before processing email"]

Added null check before processing email

**Why this approach:** ← YOU ADD THIS
Considered three options:
1. Backfill null emails with defaults - rejected (creates invalid data)
2. Disable profile for null emails - rejected (poor UX)
3. Show "Add email" prompt - chosen (guides users to fix root cause)

❌ Actual testing performed
AI can suggest tests, but doesn't know what you actually ran.

You replace AI's generic checklist with specifics:

## Testing

[AI wrote:]
- [ ] Tested happy path
- [ ] Tested edge cases
- [ ] Added unit tests

[You write:]
- [x] Tested with user ID 12345 (has null email) - shows prompt correctly
- [x] Tested with user ID 67890 (has email) - edit form works normally
- [x] Verified with @sarah-qa on staging environment
- [x] Added regression test in profile.test.tsx lines 145-167

GitHub Copilot for PR Descriptions

If you use VS Code with Copilot, you can generate PR descriptions inline:

Method 1: Use Copilot Chat

@workspace Create a PR description for my current changes.
Include problem, solution, and testing sections.

Method 2: In the GitHub PR UI

GitHub Copilot can analyze your diff automatically and suggest:

PR title
Description draft
Relevant reviewers

Pro tip: Copilot is good at titles, decent at summaries, but weak on context. Use it for the skeleton, then flesh it out.

Claude Code for PR Descriptions

Claude Code excels at analyzing codebases and providing context.

Example workflow:

# In your terminal with Claude Code
claude "Analyze my recent changes and write a PR description. 
Focus on why these changes matter and what problems they solve."

Claude Code can:

Read your entire codebase for context
Understand related files you didn't change
Identify breaking changes
Suggest migration paths

Example output:

I analyzed your changes across 12 files. This appears to be migrating 
from Redux to Zustand for state management.

## Problem
The current Redux implementation has several issues:
- 200+ lines of boilerplate (actions, reducers, selectors)
- Causes unnecessary re-renders (I see 15+ useSelector calls that 
  trigger on any state change)
- Difficult to test (requires store wrapper in tests)

## Solution  
Migrated to Zustand for cleaner state management:
- Reduced from 200 lines to 45 lines
- Selective subscriptions prevent unnecessary re-renders
- Tests simplified (no Provider needed)

I noticed you kept the old Redux store in `store/legacy/` - is this for
gradual migration?

Advanced: Iterative Refinement with AI

Don't settle for the first draft. Iterate:

Round 1: Generate basic description

Claude, here's my diff. Write a PR description.

Round 2: Add context

Good start. Add that this affects 500 users and we've had 12 support 
tickets about it. Also mention we considered backfilling emails but 
rejected it because [reasons].

Round 3: Focus on testing

Expand the testing section. I tested with users 12345 and 67890, 
verified the fix in staging, and added regression tests. Make it 
more specific.

Round 4: Add reviewer notes

Add a "Reviewer Notes" section. I want reviewers to focus on the 
null check logic in ProfileForm.tsx and the test coverage in 
profile.test.tsx.

The Hybrid Approach (Best Practice)

## My Workflow

1. **AI generates first draft** (2 minutes)
   - Paste diff into Claude
   - Get basic structure

2. **I add business context** (3 minutes)
   - Why this matters
   - Impact numbers
   - Customer feedback

3. **I add specifics** (3 minutes)  
   - Actual testing performed
   - Decisions made and why
   - Risks and concerns

4. **AI polishes** (1 minute)
   - "Claude, make this more concise"
   - "Reorganize for clarity"

**Total time:** 9 minutes
**Quality:** Better than 30 minutes writing alone

Example: AI-Assisted PR Description

What I gave AI:

Here's my diff. Write a PR description.

[paste 15-file diff for race condition fix]

What AI generated (2 minutes):

## Problem
The application experiences race conditions when rapidly navigating between 
user profiles, causing incorrect data to display.

## Solution
Implemented AbortController to cancel in-flight requests when the user 
navigates to a different profile.

## Testing
- Tested rapid navigation
- Added unit tests for cleanup
- Verified error handling

What I added (5 minutes):

## Problem
The application experiences race conditions when rapidly navigating between 
user profiles, causing incorrect data to display.

**Impact:** ← ADDED
- Reported by 8 users in last 2 weeks
- Affects power users who navigate quickly (~5% of users)  
- Privacy concern: Users might see wrong person's data

**Example scenario:** ← ADDED
1. Click Profile A (slow response, 2s)
2. Click Profile B (fast response, 0.5s)
3. B loads correctly
4. A loads and overwrites B (wrong!)

## Solution
Implemented AbortController to cancel in-flight requests when the user 
navigates to a different profile.

**Why AbortController:** ← ADDED
- Native browser API (no library needed)
- Standard approach used by React Query, SWR
- Proper way to cancel fetch requests

**Alternatives considered:** ← ADDED
1. Request IDs - too complex
2. Debounce navigation - poor UX
3. AbortController - chosen for simplicity

## Testing
- Tested rapid navigation
- Added unit tests for cleanup
- Verified error handling

**Specifically:** ← ADDED
- [x] Navigated between 10 users in 5 seconds - correct user always shown
- [x] Tested on throttled 3G network
- [x] Verified with @sarah-qa in staging
- [x] Added test in useUserProfile.test.tsx lines 145-167

## Screenshots ← ADDED ENTIRE SECTION
**Before:** Wrong user data after rapid navigation
![before](https://i.imgur.com/before.png)

**After:** Correct user always shows
![after](https://i.imgur.com/after.png)

**Demo:** https://www.loom.com/share/abc123

Fixes #1234 ← ADDED

Result: Professional PR description in 7 minutes instead of 15.

Prompts That Work

For initial draft:

Analyze this git diff and create a PR description with:
1. Problem section (why this change is needed)
2. Solution section (high-level approach)
3. Testing section (what should be tested)

Focus on user impact and business value, not implementation details.

[paste diff]

For refactoring PRs:

This is a refactoring PR. Explain:
1. What problems exist with current code
2. What the refactored approach improves
3. Why this refactor is worth doing now
4. Any migration needed for other developers

[paste diff]

For bug fixes:

This PR fixes a bug. Help me write a description that includes:
1. The exact error users experience
2. Why the bug happens (root cause)
3. How the fix solves it
4. What testing proves it's fixed

[paste diff]

For polishing:

Here's my draft PR description. Make it:
- More concise (remove unnecessary words)
- More specific (replace vague statements)
- Better organized (improve flow)
- Professional (appropriate tone)

[paste your draft]

When NOT to Trust AI

❌ Don't trust AI for:

Exact numbers
- AI might say "affects many users"
- You say "affects 500 users (0.5%)"
Testing claims
- AI might say "thoroughly tested"
- You say exactly what you tested
Business context
- AI doesn't know your customers
- AI doesn't know your roadmap
- AI doesn't know internal discussions
Security implications
- AI might miss that you're exposing data
- AI might not flag authentication changes
- Always review security-related descriptions carefully

✅ Do trust AI for:

Code summaries - What changed across files
Test suggestions - What you should test
Structure - Organizing your thoughts
First draft - Getting started quickly

The AI Safety Checklist

Before submitting an AI-generated PR description: ""

**Review these:**

- [ ] Business context is accurate (not AI assumptions)
- [ ] Numbers are real (not AI estimates)
- [ ] Testing reflects what you ACTUALLY did
- [ ] No hallucinated details (AI making things up)
- [ ] Security implications are addressed
- [ ] Links are correct (AI might invent URLs)
- [ ] Removed AI hedging language ("might", "could", "possibly")
- [ ] Tone is appropriate (not too formal or casual)

Real Example: AI Mistakes to Fix

AI generated:

## Testing
The changes have been thoroughly tested across multiple scenarios and 
edge cases. All tests pass successfully.

Problems:

Vague ("thoroughly tested")
No specifics
Sounds like AI boilerplate

You fix it:

## Testing

**Manual:**
- [x] Tested with user ID 12345 (null email) - prompt shows correctly
- [x] Tested with user ID 67890 (valid email) - form works normally
- [x] Rapid clicking "Add email" button - no issues

**Automated:**
- [x] Added unit test: `profile.test.tsx` lines 145-167
- [x] All existing tests pass (87/87)
- [x] Coverage increased from 78% to 82%

Tools Comparison

Tool	Best For	Limitations
Claude	Deep analysis, context understanding, refinement	No direct Git integration
Claude Code	Full codebase context, terminal integration	Requires Claude subscription
GitHub Copilot	In-IDE suggestions, PR UI integration	Less context, shorter responses
ChatGPT	General descriptions, formatting help	No code context by default

My recommendation: Use Claude or Claude Code for complex PRs (refactors, large features). Use Copilot for quick bug fixes and small changes.

The Bottom Line on AI

AI is a 10x multiplier for PR descriptions:

15 minutes → 5 minutes with better quality
Blank page problem solved (AI gives you a start)
Catches things you forgot (suggested tests, edge cases)

But AI is not a replacement:

You still need to add context
You still need to verify accuracy
You still need to add human judgment

Best practice:

AI writes the skeleton.
You add the soul.

Use AI to:

✅ Generate structure quickly
✅ Summarize code changes
✅ Suggest test cases
✅ Polish your writing

Don't use AI to:

❌ Replace your thinking
❌ Make up business context
❌ Claim testing you didn't do
❌ Skip the review process

The workflow that works:

Let AI generate first draft (2 min)
Add your knowledge (5 min)
Let AI polish (1 min)

Total: 8 minutes for a great PR description.

Conclusion

Good PR descriptions:

✅ Save reviewers time
✅ Get faster approvals
✅ Reduce back-and-forth
✅ Prevent bugs from slipping through
✅ Create better documentation

Bad PR descriptions:

❌ Waste reviewers' time
❌ Sit for days
❌ Require multiple review rounds
❌ Let bugs slip through
❌ Leave no documentation

The formula:

Problem (why) + Solution (what) + Testing (proof) = Fast approval

My challenge to you:

Tomorrow, write a PR description using this template. Time yourself:

How long to write: ~10 minutes
How long to get approved: Probably 10x faster

That 10 minutes saves everyone hours.

What's your PR description template? Share your best practices!

Setting Up TypeScript ESLint Rules Teams Actually Follow

Tarun Moorjani — Mon, 02 Mar 2026 21:03:47 +0000

Setting Up TypeScript ESLint Rules Teams Actually Follow

I've seen two types of ESLint configs:

Type 1: The Abandoned Config

{
  "rules": {
    "no-console": "error",
    "@typescript-eslint/no-explicit-any": "error",
    "@typescript-eslint/explicit-function-return-type": "error",
    "react/prop-types": "error"
  }
}

Result: Developers disable ESLint entirely because it's "too annoying." The codebase has // eslint-disable comments everywhere. The config sits unused.

Type 2: The "Whatever" Config

{
  "extends": ["eslint:recommended"]
}

Result: ESLint catches nothing useful. Bugs slip through. The team wonders why they even have ESLint.

After 5 years of tuning ESLint configs across multiple teams, I've found the sweet spot: rules that prevent real bugs without driving developers crazy.

Here's the config that actually works.

The Philosophy: Three Tiers

Tier 1: Rules That Catch Bugs (Always Enable)

These prevent production incidents. Never disable them.

Tier 2: Rules That Improve Quality (Enable, But Configurable)

These catch code smells and maintainability issues. Worth enabling, but might need team discussion.

Tier 3: Rules That Annoy Developers (Usually Disable)

These enforce style preferences that don't prevent bugs. Let Prettier handle formatting.

The Base Configuration

Start here. This works for 90% of teams.

npm install --save-dev \
  eslint \
  @typescript-eslint/eslint-plugin \
  @typescript-eslint/parser \
  eslint-plugin-react \
  eslint-plugin-react-hooks \
  eslint-plugin-jsx-a11y \
  eslint-config-prettier

// .eslintrc.json
{
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "ecmaVersion": 2022,
    "sourceType": "module",
    "ecmaFeatures": {
      "jsx": true
    },
    "project": "./tsconfig.json"
  },
  "plugins": [
    "@typescript-eslint",
    "react",
    "react-hooks",
    "jsx-a11y"
  ],
  "extends": [
    "eslint:recommended",
    "plugin:@typescript-eslint/recommended",
    "plugin:react/recommended",
    "plugin:react-hooks/recommended",
    "plugin:jsx-a11y/recommended",
    "prettier" // Must be last
  ],
  "rules": {
    // Our custom rules go here
  },
  "settings": {
    "react": {
      "version": "detect"
    }
  }
}

Tier 1: Bug-Preventing Rules (Always Enable)

These rules have caught real bugs in production. Never disable them.

1. `@typescript-eslint/no-floating-promises`

What it catches:

// ❌ Promise rejection unhandled - app crashes
async function deleteUser(id: string) {
  await api.delete(`/users/${id}`);
}

function handleClick() {
  deleteUser(userId); // Forgot await! Error disappears silently
}

// ✅ Forces you to handle it
async function handleClick() {
  await deleteUser(userId);
  // or
  deleteUser(userId).catch(handleError);
  // or explicitly ignore
  void deleteUser(userId);
}

Configuration:

{
  "rules": {
    "@typescript-eslint/no-floating-promises": "error"
  }
}

Why it matters: Unhandled promise rejections crash Node.js and cause silent failures in browsers.

2. `@typescript-eslint/no-misused-promises`

What it catches:

// ❌ Async function in event handler - doesn't wait
<button onClick={async () => {
  await saveData();
  console.log('Saved!'); // This runs, but React doesn't wait
}}>Save</button>

// ❌ Using promise in conditional
if (fetchData()) { // Always truthy! fetchData returns Promise
  // This always runs, even if fetch fails
}

// ✅ Correct
const handleClick = async () => {
  await saveData();
  console.log('Saved!');
};

<button onClick={handleClick}>Save</button>

// ✅ Correct
const data = await fetchData();
if (data) {
  // Now checking actual data
}

Configuration:

{
  "rules": {
    "@typescript-eslint/no-misused-promises": [
      "error",
      {
        "checksVoidReturn": true,
        "checksConditionals": true
      }
    ]
  }
}

3. `react-hooks/exhaustive-deps`

What it catches:

// ❌ Stale closure bug
function Component({ userId }: { userId: string }) {
  const [user, setUser] = useState(null);

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

  // User never updates when userId changes
}

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

Configuration:

{
  "rules": {
    "react-hooks/exhaustive-deps": "error"
  }
}

Why it matters: Stale closures are the #1 cause of bugs in hooks-based React apps.

4. `@typescript-eslint/no-unnecessary-type-assertion`

What it catches:

// ❌ Lying to TypeScript
const value = apiResponse as User; // No validation!

// If API shape changes, TypeScript can't help
value.newProperty; // Runtime error

// ✅ Use type guards instead
function isUser(value: unknown): value is User {
  return (
    typeof value === 'object' &&
    value !== null &&
    'id' in value &&
    'name' in value
  );
}

if (isUser(apiResponse)) {
  apiResponse.name; // Safe
}

Configuration:

{
  "rules": {
    "@typescript-eslint/no-unnecessary-type-assertion": "error"
  }
}

5. `@typescript-eslint/await-thenable`

What it catches:

// ❌ Awaiting non-promise
async function example() {
  await nonAsyncFunction(); // Does nothing, misleading
  const value = await synchronousValue; // Not a promise
}

// ✅ Only await actual promises
async function example() {
  nonAsyncFunction(); // No await needed
  const value = synchronousValue; // No await needed
  await actualAsyncFunction(); // This needs await
}

Configuration:

{
  "rules": {
    "@typescript-eslint/await-thenable": "error"
  }
}

6. `@typescript-eslint/no-unsafe-assignment`

What it catches:

// ❌ Unsafe assignment from any
function processData(data: any) {
  const user: User = data; // No validation!
  user.email.toLowerCase(); // Crashes if data.email is undefined
}

// ✅ Validate before assigning
function processData(data: unknown) {
  const validated = UserSchema.parse(data);
  validated.email.toLowerCase(); // Safe
}

Configuration:

{
  "rules": {
    "@typescript-eslint/no-unsafe-assignment": "warn", // Warn first, error later
    "@typescript-eslint/no-unsafe-member-access": "warn",
    "@typescript-eslint/no-unsafe-call": "warn",
    "@typescript-eslint/no-unsafe-return": "warn"
  }
}

Note: Start with "warn" because existing code might have many violations. Upgrade to "error" over time.

7. `@typescript-eslint/no-unused-vars`

What it catches:

// ❌ Dead code
function calculate(a: number, b: number, c: number) {
  return a + b; // c is never used - probably a bug
}

// ❌ Unused imports slow down bundle
import { ComponentA, ComponentB, ComponentC } from './components';
// Only using ComponentA

// ✅ Clean code
function calculate(a: number, b: number) {
  return a + b;
}

import { ComponentA } from './components';

Configuration:

{
  "rules": {
    "@typescript-eslint/no-unused-vars": [
      "error",
      {
        "argsIgnorePattern": "^_",
        "varsIgnorePattern": "^_",
        "caughtErrorsIgnorePattern": "^_"
      }
    ]
  }
}

Pro tip: Use _ prefix for intentionally unused variables:

function onClick(_event: MouseEvent) {
  // Don't need event, but required by interface
}

8. `react-hooks/rules-of-hooks`

What it catches:

// ❌ Conditional hooks
function Component({ condition }) {
  if (condition) {
    const data = useFetch('/api/data'); // Breaks rules of hooks
  }
}

// ❌ Hooks in loops
items.map(item => {
  const data = useFetch(item.url); // Breaks rules of hooks
});

// ✅ Hooks at top level
function Component({ condition }) {
  const data = useFetch(condition ? '/api/data' : null);
}

Configuration:

{
  "rules": {
    "react-hooks/rules-of-hooks": "error"
  }
}

Tier 2: Quality-Improving Rules (Recommended)

These improve code quality but might need discussion with your team.

1. `@typescript-eslint/no-explicit-any`

The debate:

// Some teams: "any is never okay"
// Other teams: "any is fine for prototypes"

My recommendation: Warn, not error. Allow any with justification.

{
  "rules": {
    "@typescript-eslint/no-explicit-any": "warn"
  }
}

When any is acceptable:

Working with truly dynamic data (e.g., JSON.parse)
Prototyping (temporary)
Interfacing with untyped libraries (until you add types)

Add a comment when you use any:

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const data: any = JSON.parse(response);
// TODO: Add proper type validation with Zod

2. `@typescript-eslint/no-non-null-assertion`

What it catches:

// ❌ Risky non-null assertion
const user = users.find(u => u.id === userId)!;
user.name; // Crashes if user not found

// ✅ Proper null handling
const user = users.find(u => u.id === userId);
if (!user) throw new Error('User not found');
user.name; // Safe

Configuration:

{
  "rules": {
    "@typescript-eslint/no-non-null-assertion": "warn"
  }
}

When ! is acceptable:

After explicit null checks
In tests where you control the data
When you're 100% certain (document why)

// Acceptable use
const element = document.getElementById('root');
if (!element) throw new Error('Root element not found');

// Now we're certain
ReactDOM.render(<App />, element!);

3. `@typescript-eslint/explicit-function-return-type`

The debate: Should functions require explicit return types?

My take: No for private functions, yes for public APIs.

{
  "rules": {
    "@typescript-eslint/explicit-function-return-type": "off",
    "@typescript-eslint/explicit-module-boundary-types": "warn"
  }
}

Why:

// ❌ Too verbose for internal code
const add = (a: number, b: number): number => a + b; // Obvious return type

// ✅ Let inference work
const add = (a: number, b: number) => a + b; // TypeScript infers number

// ✅ Explicit for public APIs
export function processUser(user: User): ProcessedUser {
  // Return type is part of the contract
  return { ...user, processed: true };
}

4. `@typescript-eslint/consistent-type-imports`

What it does:

// Before
import { User } from './types';
import { api } from './api';

// After
import type { User } from './types';
import { api } from './api';

Why it matters:

Smaller bundles (type imports are stripped)
Clearer separation of types vs values
Faster TypeScript compilation

Configuration:

{
  "rules": {
    "@typescript-eslint/consistent-type-imports": [
      "warn",
      {
        "prefer": "type-imports",
        "fixable": true
      }
    ]
  }
}

5. `@typescript-eslint/prefer-nullish-coalescing`

What it catches:

// ❌ Logical OR treats 0, '', false as falsy
const count = userInput || 0; // If user enters 0, returns 0 (wrong!)
const name = user.name || 'Anonymous'; // If name is '', returns 'Anonymous'

// ✅ Nullish coalescing only treats null/undefined as falsy
const count = userInput ?? 0; // If user enters 0, returns 0 (correct!)
const name = user.name ?? 'Anonymous'; // Only uses default if null/undefined

Configuration:

{
  "rules": {
    "@typescript-eslint/prefer-nullish-coalescing": "warn"
  }
}

6. `@typescript-eslint/prefer-optional-chain`

What it catches:

// ❌ Old way
const email = user && user.profile && user.profile.email;

// ✅ Optional chaining
const email = user?.profile?.email;

Configuration:

{
  "rules": {
    "@typescript-eslint/prefer-optional-chain": "warn"
  }
}

7. `jsx-a11y/*` (Accessibility Rules)

Start with recommended, customize as needed:

{
  "extends": [
    "plugin:jsx-a11y/recommended"
  ],
  "rules": {
    // Enforce label association
    "jsx-a11y/label-has-associated-control": "error",

    // Require alt text
    "jsx-a11y/alt-text": "error",

    // Warn on missing ARIA labels (not always needed)
    "jsx-a11y/aria-label": "warn",

    // Some rules are too strict for real apps
    "jsx-a11y/no-autofocus": "off", // Sometimes you DO want autofocus
    "jsx-a11y/click-events-have-key-events": "warn" // Warn, don't block
  }
}

Tier 3: Rules to Disable (Annoying Without Value)

These rules sound good in theory but cause frustration in practice.

1. `@typescript-eslint/explicit-function-return-type` (Too Verbose)

// ❌ This rule forces:
const add = (a: number, b: number): number => a + b;

// ✅ TypeScript already knows the return type
const add = (a: number, b: number) => a + b;

Disable it:

{
  "rules": {
    "@typescript-eslint/explicit-function-return-type": "off"
  }
}

2. `react/prop-types` (Redundant with TypeScript)

// ❌ This rule forces PropTypes when you have TypeScript
interface ButtonProps {
  label: string;
  onClick: () => void;
}

function Button({ label, onClick }: ButtonProps) {
  return <button onClick={onClick}>{label}</button>;
}

Button.propTypes = { // Redundant!
  label: PropTypes.string.isRequired,
  onClick: PropTypes.func.isRequired,
};

Disable it:

{
  "rules": {
    "react/prop-types": "off"
  }
}

3. `@typescript-eslint/naming-convention` (Too Opinionated)

This rule tries to enforce naming patterns. In practice, it causes more arguments than it's worth.

{
  "rules": {
    "@typescript-eslint/naming-convention": "off"
  }
}

Why: Naming is subjective. Let code review handle it.

4. `no-console` (Too Aggressive)

// ❌ Blocks legitimate debugging
console.log('User logged in:', userId);
console.error('Payment failed:', error);

Better approach: Allow console, but remove before production

{
  "rules": {
    "no-console": "off" // Or use a build step to strip console.logs
  }
}

5. `@typescript-eslint/ban-ts-comment` (Too Strict)

Sometimes you need @ts-ignore for legitimate reasons (working around library bugs, etc.)

{
  "rules": {
    "@typescript-eslint/ban-ts-comment": [
      "warn",
      {
        "ts-ignore": "allow-with-description",
        "minimumDescriptionLength": 10
      }
    ]
  }
}

This allows @ts-ignore but requires explanation:

// @ts-ignore: Library types are wrong, PR submitted to DefinitelyTyped
const result = libraryFunction(param);

The Complete Configuration

Here's the full config that balances safety and DX:

{
  "parser": "@typescript-eslint/parser",
  "parserOptions": {
    "ecmaVersion": 2022,
    "sourceType": "module",
    "ecmaFeatures": {
      "jsx": true
    },
    "project": "./tsconfig.json"
  },
  "plugins": [
    "@typescript-eslint",
    "react",
    "react-hooks",
    "jsx-a11y"
  ],
  "extends": [
    "eslint:recommended",
    "plugin:@typescript-eslint/recommended",
    "plugin:react/recommended",
    "plugin:react-hooks/recommended",
    "plugin:jsx-a11y/recommended",
    "prettier"
  ],
  "rules": {
    // === Tier 1: Bug Prevention (Always Error) ===
    "@typescript-eslint/no-floating-promises": "error",
    "@typescript-eslint/no-misused-promises": "error",
    "@typescript-eslint/await-thenable": "error",
    "@typescript-eslint/no-unnecessary-type-assertion": "error",
    "react-hooks/rules-of-hooks": "error",
    "react-hooks/exhaustive-deps": "error",
    "@typescript-eslint/no-unused-vars": [
      "error",
      {
        "argsIgnorePattern": "^_",
        "varsIgnorePattern": "^_"
      }
    ],

    // === Tier 2: Quality Improvement (Warn) ===
    "@typescript-eslint/no-explicit-any": "warn",
    "@typescript-eslint/no-non-null-assertion": "warn",
    "@typescript-eslint/no-unsafe-assignment": "warn",
    "@typescript-eslint/no-unsafe-member-access": "warn",
    "@typescript-eslint/no-unsafe-call": "warn",
    "@typescript-eslint/prefer-nullish-coalescing": "warn",
    "@typescript-eslint/prefer-optional-chain": "warn",
    "@typescript-eslint/consistent-type-imports": [
      "warn",
      { "prefer": "type-imports" }
    ],

    // === Tier 3: Disable Annoying Rules ===
    "@typescript-eslint/explicit-function-return-type": "off",
    "@typescript-eslint/explicit-module-boundary-types": "off",
    "react/prop-types": "off",
    "react/react-in-jsx-scope": "off", // Not needed in React 17+
    "no-console": "off",

    // === Accessibility (Customize for your app) ===
    "jsx-a11y/no-autofocus": "off",
    "jsx-a11y/click-events-have-key-events": "warn"
  },
  "settings": {
    "react": {
      "version": "detect"
    }
  }
}

Performance Optimization

ESLint can be slow on large codebases. Optimize it:

1. Use Type-Aware Rules Selectively

{
  "parserOptions": {
    // Only use project for files that need type-aware rules
    "project": "./tsconfig.json"
  },
  "overrides": [
    {
      // Disable type-aware rules for test files
      "files": ["**/*.test.ts", "**/*.test.tsx"],
      "rules": {
        "@typescript-eslint/no-floating-promises": "off"
      }
    }
  ]
}

2. Ignore Generated Files

// .eslintignore
node_modules/
dist/
build/
*.config.js
coverage/
.next/

3. Use Caching

// package.json
{
  "scripts": {
    "lint": "eslint . --cache --cache-location .eslintcache"
  }
}

Integration with Prettier

ESLint and Prettier should complement, not fight:

npm install --save-dev eslint-config-prettier

{
  "extends": [
    // ... other extends
    "prettier" // MUST be last
  ]
}

What Prettier handles:

Semicolons
Quotes
Indentation
Line length
Trailing commas

What ESLint handles:

Code quality
Bug prevention
Best practices

Never configure ESLint for formatting. Let Prettier do that.

Gradual Adoption Strategy

Don't enable all rules at once. Roll them out gradually:

Phase 1: Foundation (Week 1)

{
  "extends": [
    "eslint:recommended",
    "plugin:@typescript-eslint/recommended"
  ]
}

Phase 2: React Rules (Week 2-3)

{
  "extends": [
    "eslint:recommended",
    "plugin:@typescript-eslint/recommended",
    "plugin:react/recommended",
    "plugin:react-hooks/recommended"
  ]
}

Phase 3: Strict Type Safety (Week 4-6)

{
  "rules": {
    "@typescript-eslint/no-floating-promises": "error",
    "@typescript-eslint/no-misused-promises": "error"
  }
}

Phase 4: Quality Rules (Week 7-8)

{
  "rules": {
    "@typescript-eslint/no-explicit-any": "warn",
    "@typescript-eslint/prefer-nullish-coalescing": "warn"
  }
}

Phase 5: Accessibility (Week 9+)

{
  "extends": [
    "plugin:jsx-a11y/recommended"
  ]
}

Handling Existing Violations

When adding new rules to existing codebases:

Option 1: Fix Incrementally

{
  "rules": {
    "@typescript-eslint/no-explicit-any": "warn" // Start with warning
  }
}

Fix warnings over time. Upgrade to "error" when all fixed.

Option 2: Grandfather Existing Code

{
  "overrides": [
    {
      "files": ["src/legacy/**/*"],
      "rules": {
        "@typescript-eslint/no-explicit-any": "off"
      }
    }
  ]
}

New code follows rules. Legacy code exempted (for now).

Option 3: Set Max Warnings

// package.json
{
  "scripts": {
    "lint": "eslint . --max-warnings 100"
  }
}

Allow existing warnings, but don't allow new ones.

VSCode Integration

Make ESLint fix issues on save:

// .vscode/settings.json
{
  "editor.codeActionsOnSave": {
    "source.fixAll.eslint": true
  },
  "editor.formatOnSave": true,
  "editor.defaultFormatter": "esbenp.prettier-vscode",
  "eslint.validate": [
    "javascript",
    "javascriptreact",
    "typescript",
    "typescriptreact"
  ]
}

CI/CD Integration

Block PRs with lint errors:

# .github/workflows/lint.yml
name: Lint

on: [pull_request]

jobs:
  lint:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-node@v3
      - run: npm ci
      - run: npm run lint

Pro tip: Fail on errors, allow warnings:

{
  "scripts": {
    "lint": "eslint .",
    "lint:ci": "eslint . --max-warnings 0"
  }
}

Use lint locally (warnings OK), lint:ci in CI (no warnings).

Common Pitfalls

Pitfall 1: Too Many Rules at Once

Problem: Team gets overwhelmed, disables ESLint entirely.

Solution: Add rules gradually. Measure before/after.

Pitfall 2: Treating Warnings as Noise

Problem: 1000+ warnings. Team ignores them all.

Solution: Either fix warnings or make them errors. Warnings should be temporary.

Pitfall 3: Not Explaining Why

Problem: Team doesn't understand why rules exist.

Solution: Document your rules with links to examples of bugs they prevent.

Pitfall 4: Inconsistent Application

Problem: Rules enforced in CI but not locally.

Solution: Use git hooks to run ESLint pre-commit.

npm install --save-dev husky lint-staged

// package.json
{
  "lint-staged": {
    "*.{ts,tsx}": ["eslint --fix", "prettier --write"]
  }
}

Measuring Success

Track these metrics:

Before ESLint improvements:

Production bugs per week
Code review comments about bugs
Time spent debugging

After ESLint improvements:

Same metrics
ESLint violations caught
Developer satisfaction (survey)

Success looks like:

Fewer production bugs
Faster code reviews
Developers saying "ESLint caught this before I committed"

The Rules Summary

Always Enable (Error)

@typescript-eslint/no-floating-promises
@typescript-eslint/no-misused-promises
@typescript-eslint/await-thenable
react-hooks/rules-of-hooks
react-hooks/exhaustive-deps
@typescript-eslint/no-unused-vars

Usually Enable (Warn)

@typescript-eslint/no-explicit-any
@typescript-eslint/no-non-null-assertion
@typescript-eslint/prefer-nullish-coalescing
@typescript-eslint/prefer-optional-chain
@typescript-eslint/consistent-type-imports

Usually Disable

@typescript-eslint/explicit-function-return-type
react/prop-types
no-console
@typescript-eslint/naming-convention

Conclusion

Good ESLint configuration is about signal over noise.

The goal isn't to enforce style. The goal is to prevent bugs and improve code quality without annoying developers.

Start conservative:

Enable bug-preventing rules first
Add quality rules gradually
Disable annoying rules
Let Prettier handle formatting

Measure results:

Fewer bugs?
Faster reviews?
Happier developers?

If ESLint is catching bugs before they ship, it's working. If developers are disabling it, it's not.

The best ESLint config is the one your team actually uses.

What ESLint rules do you love or hate? Share your config in the comments!

Code Review for TypeScript React: What to Look For

Tarun Moorjani — Tue, 24 Feb 2026 03:11:15 +0000

I've reviewed over 1,000 React PRs. Early on, I nitpicked everything: indentation, variable names, comment styles. My reviews were long, exhausting, and ineffective. Developers got defensive. Important issues slipped through while I argued about semicolons.

Then I learned to distinguish between what matters and what doesn't. I started catching bugs that would have shipped. I helped junior developers grow. My reviews became shorter but more valuable.

Here's what I look for now—organized by impact. Not style guides. Not preferences. The things that actually matter.

The Review Priority Pyramid

High Priority (Must Fix)
├─ Security vulnerabilities
├─ Type safety violations
├─ Logic bugs
└─ Performance issues

Medium Priority (Should Fix)
├─ Architecture concerns
├─ Accessibility issues
├─ Missing tests
└─ Poor error handling

Low Priority (Nice to Have)
├─ Code organization
├─ Naming improvements
└─ Minor optimizations

Ignore
├─ Personal style preferences
├─ Subjective opinions
└─ Bikeshedding

Start at the top. Work down. If you're commenting on indentation before type safety, you're doing it wrong.

Category 1: Type Safety (High Priority)

Red Flag: Liberal `any` Usage

// ❌ BLOCK: Defeats TypeScript's purpose
function handleSubmit(data: any) {
  // What shape is data? Who knows!
  api.post('/users', data);
}

const user: any = fetchUser();
user.nonExistentProperty; // No error, runtime bomb

// ✅ APPROVE: Proper typing
interface FormData {
  name: string;
  email: string;
}

function handleSubmit(data: FormData) {
  api.post('/users', data);
}

interface User {
  id: string;
  name: string;
}

const user: User = fetchUser();

Review comment template:

⚠️ Using `any` here removes all type safety. Could we type this as:

interface FormData {
  name: string;
  email: string;
}

This would catch errors at compile time instead of runtime.

Red Flag: Type Assertions (`as`) Without Justification

// ❌ QUESTION: Why is this safe?
const value = unknownData as ComplexType;

// ❌ BLOCK: Lying to TypeScript
const response = await fetch('/api/user');
const user = (await response.json()) as User; // What if server returns different shape?

// ✅ APPROVE: Runtime validation
import { z } from 'zod';

const UserSchema = z.object({
  id: z.string(),
  name: z.string(),
  email: z.string().email(),
});

const response = await fetch('/api/user');
const data = await response.json();
const user = UserSchema.parse(data); // Throws if invalid

Review comment:

🤔 This type assertion looks risky. Can we add runtime validation?

const UserSchema = z.object({...});
const user = UserSchema.parse(data);

Or if we're confident about the API shape, add a comment explaining why the assertion is safe.

Red Flag: Missing Null/Undefined Checks

// ❌ BLOCK: Potential null reference error
function UserProfile({ user }: { user: User | null }) {
  return <div>{user.name}</div>; // Crashes if user is null
}

// ❌ QUESTION: Is this safe?
const firstUser = users[0];
console.log(firstUser.name); // What if array is empty?

// ✅ APPROVE: Proper null handling
function UserProfile({ user }: { user: User | null }) {
  if (!user) return <div>No user found</div>;
  return <div>{user.name}</div>;
}

const firstUser = users[0];
if (firstUser) {
  console.log(firstUser.name);
}

// Or use optional chaining
console.log(users[0]?.name);

Red Flag: Improper Type Narrowing

// ❌ QUESTION: This doesn't narrow the type
function processValue(value: string | number) {
  if (value) { // Doesn't narrow
    return value.toFixed(2); // Error: string doesn't have toFixed
  }
}

// ✅ APPROVE: Proper type guard
function processValue(value: string | number) {
  if (typeof value === 'number') {
    return value.toFixed(2);
  }
  return value;
}

Category 2: React Patterns (High Priority)

Red Flag: Breaking Rules of Hooks

// ❌ BLOCK: Conditional hook usage
function Component({ condition }: { condition: boolean }) {
  if (condition) {
    const data = useFetch('/api/data'); // Hooks must be unconditional
  }
}

// ❌ BLOCK: Hook in loop
function Component({ items }: { items: Item[] }) {
  return items.map(item => {
    const data = useFetch(item.url); // Can't call hooks in loops
  });
}

// ✅ APPROVE: Hooks at top level
function Component({ condition }: { condition: boolean }) {
  const data = useFetch(condition ? '/api/data' : null);

  if (!condition) return null;
  // Use data
}

Red Flag: Missing Dependencies in useEffect/useCallback

// ❌ BLOCK: Stale closure bug
function Component({ userId }: { userId: string }) {
  const [user, setUser] = useState(null);

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

  // User won't update when userId changes
}

// ✅ APPROVE: Correct dependencies
useEffect(() => {
  fetchUser(userId).then(setUser);
}, [userId]);

// ✅ ALSO GOOD: Acknowledge ESLint disable
useEffect(() => {
  // Only fetch on mount, ignoring userId changes
  // eslint-disable-next-line react-hooks/exhaustive-deps
  fetchUser(userId).then(setUser);
}, []);

Review comment:

⚠️ `userId` is used in the effect but not in the dependency array. This means the effect won't re-run when `userId` changes, causing stale data.

Add `userId` to dependencies:
useEffect(() => {
  fetchUser(userId).then(setUser);
}, [userId]);

Red Flag: Unnecessary Re-renders

// ❌ QUESTION: This re-renders on every parent render
function Parent() {
  return (
    <Child 
      onClick={() => console.log('clicked')} 
      style={{ margin: 10 }}
      config={{ theme: 'dark' }}
    />
  );
}

const Child = React.memo(({ onClick, style, config }) => {
  // Child re-renders every time because new objects/functions
});

// ✅ APPROVE: Stable references
function Parent() {
  const handleClick = useCallback(() => {
    console.log('clicked');
  }, []);

  const style = useMemo(() => ({ margin: 10 }), []);

  const config = useMemo(() => ({ theme: 'dark' }), []);

  return <Child onClick={handleClick} style={style} config={config} />;
}

// ✅ EVEN BETTER: Move static data outside component
const STYLE = { margin: 10 };
const CONFIG = { theme: 'dark' };

function Parent() {
  const handleClick = useCallback(() => {
    console.log('clicked');
  }, []);

  return <Child onClick={handleClick} style={STYLE} config={CONFIG} />;
}

Red Flag: State Derived from Props

// ❌ QUESTION: This gets out of sync
function Component({ initialCount }: { initialCount: number }) {
  const [count, setCount] = useState(initialCount);

  // If initialCount changes, count stays the same!
  return <div>{count}</div>;
}

// ✅ APPROVE: Use the prop directly
function Component({ count }: { count: number }) {
  return <div>{count}</div>;
}

// ✅ OR: Use key to reset state when prop changes
<Component key={initialCount} initialCount={initialCount} />

// ✅ OR: Sync with useEffect (if you must)
function Component({ initialCount }: { initialCount: number }) {
  const [count, setCount] = useState(initialCount);

  useEffect(() => {
    setCount(initialCount);
  }, [initialCount]);

  return <div>{count}</div>;
}

Red Flag: Mutating State Directly

// ❌ BLOCK: Direct mutation
function Component() {
  const [items, setItems] = useState<Item[]>([]);

  const addItem = (item: Item) => {
    items.push(item); // Mutation!
    setItems(items); // React won't detect the change
  };
}

// ✅ APPROVE: Immutable update
const addItem = (item: Item) => {
  setItems([...items, item]);
};

// ✅ APPROVE: Functional update
const addItem = (item: Item) => {
  setItems(prev => [...prev, item]);
};

Category 3: Performance Issues (High Priority)

Red Flag: Expensive Operations in Render

// ❌ QUESTION: This runs on every render
function Component({ items }: { items: Item[] }) {
  const sorted = items
    .filter(item => item.active)
    .sort((a, b) => b.priority - a.priority)
    .map(item => ({ ...item, formatted: formatItem(item) }));

  return <List items={sorted} />;
}

// ✅ APPROVE: Memoized computation
function Component({ items }: { items: Item[] }) {
  const sorted = useMemo(() => 
    items
      .filter(item => item.active)
      .sort((a, b) => b.priority - a.priority)
      .map(item => ({ ...item, formatted: formatItem(item) })),
    [items]
  );

  return <List items={sorted} />;
}

Red Flag: Large Bundle Imports

// ❌ QUESTION: Importing entire library
import _ from 'lodash'; // Bundles all of lodash
import * as dateFns from 'date-fns'; // Bundles all of date-fns

_.debounce(fn, 100);
dateFns.format(date, 'yyyy-MM-dd');

// ✅ APPROVE: Import only what you need
import debounce from 'lodash/debounce';
import { format } from 'date-fns';

debounce(fn, 100);
format(date, 'yyyy-MM-dd');

Red Flag: Inefficient List Rendering

// ❌ BLOCK: Missing keys or bad keys
items.map((item, index) => (
  <Item key={index} {...item} /> // Index as key is bad
));

items.map(item => (
  <Item {...item} /> // No key at all
));

// ✅ APPROVE: Stable, unique keys
items.map(item => (
  <Item key={item.id} {...item} />
));

Category 4: Security (High Priority)

Red Flag: XSS Vulnerabilities

// ❌ BLOCK: XSS vulnerability
function Comment({ comment }: { comment: string }) {
  return <div dangerouslySetInnerHTML={{ __html: comment }} />;
  // User can inject <script>alert('XSS')</script>
}

// ✅ APPROVE: Sanitize HTML
import DOMPurify from 'dompurify';

function Comment({ comment }: { comment: string }) {
  const sanitized = DOMPurify.sanitize(comment);
  return <div dangerouslySetInnerHTML={{ __html: sanitized }} />;
}

// ✅ BETTER: Use markdown or text
function Comment({ comment }: { comment: string }) {
  return <div>{comment}</div>; // React escapes by default
}

Red Flag: Exposing Sensitive Data

// ❌ BLOCK: Leaking API keys
const API_KEY = 'sk-123456789'; // Hardcoded in frontend
fetch(`https://api.com?key=${API_KEY}`);

// ❌ BLOCK: Logging sensitive data
console.log('User password:', password);
console.log('Credit card:', creditCard);

// ✅ APPROVE: Use environment variables (server-side)
// This should be in backend, not frontend
const response = await fetch('/api/proxy', {
  headers: { 'Authorization': 'Bearer server-side-token' }
});

// ✅ APPROVE: Never log sensitive data
console.log('User logged in:', user.id); // ID only, no sensitive data

Red Flag: Insecure Data Handling

// ❌ QUESTION: Is this input sanitized?
function SearchResults({ query }: { query: string }) {
  const results = await fetch(`/api/search?q=${query}`);
  // What if query contains SQL injection attempts?
}

// ✅ APPROVE: Use URL encoding
function SearchResults({ query }: { query: string }) {
  const encodedQuery = encodeURIComponent(query);
  const results = await fetch(`/api/search?q=${encodedQuery}`);
}

Category 5: Architecture & Design (Medium Priority)

Red Flag: God Components

// ❌ QUESTION: 500+ lines, does everything
function Dashboard() {
  // Fetches 10 different data sources
  // Handles authentication
  // Manages complex state
  // Renders everything

  return (
    <div>
      {/* 400 lines of JSX */}
    </div>
  );
}

// ✅ APPROVE: Decomposed
function Dashboard() {
  return (
    <DashboardLayout>
      <DashboardHeader />
      <DashboardStats />
      <DashboardCharts />
      <DashboardActivity />
    </DashboardLayout>
  );
}

Review comment:

💡 This component is doing a lot. Consider breaking it into smaller components:

- DashboardHeader (user info, navigation)
- DashboardStats (metrics cards)
- DashboardCharts (visualizations)
- DashboardActivity (recent activity)

Each can manage its own data fetching and state.

Red Flag: Prop Drilling

// ❌ QUESTION: Passing props through 5 levels
function App() {
  const user = useAuth();
  return <Layout user={user} />;
}

function Layout({ user }) {
  return <Sidebar user={user} />;
}

function Sidebar({ user }) {
  return <Navigation user={user} />;
}

function Navigation({ user }) {
  return <UserMenu user={user} />;
}

function UserMenu({ user }) {
  return <div>{user.name}</div>;
}

// ✅ APPROVE: Context or custom hook
const UserContext = createContext<User | null>(null);

function App() {
  const user = useAuth();
  return (
    <UserContext.Provider value={user}>
      <Layout />
    </UserContext.Provider>
  );
}

function UserMenu() {
  const user = useContext(UserContext);
  return <div>{user?.name}</div>;
}

Red Flag: Mixed Concerns

// ❌ QUESTION: Business logic in component
function ProductList() {
  const [products, setProducts] = useState([]);

  useEffect(() => {
    fetch('/api/products')
      .then(res => res.json())
      .then(data => {
        // Complex business logic
        const processed = data
          .filter(p => p.stock > 0)
          .map(p => ({
            ...p,
            discountedPrice: calculateDiscount(p),
            shipping: calculateShipping(p),
            taxes: calculateTaxes(p),
          }))
          .sort((a, b) => b.popularity - a.popularity);

        setProducts(processed);
      });
  }, []);

  return <div>{/* render */}</div>;
}

// ✅ APPROVE: Extract to custom hook
function useProducts() {
  const [products, setProducts] = useState([]);

  useEffect(() => {
    fetch('/api/products')
      .then(res => res.json())
      .then(data => {
        const processed = processProducts(data);
        setProducts(processed);
      });
  }, []);

  return products;
}

// Business logic in pure function (testable!)
function processProducts(products: Product[]): ProcessedProduct[] {
  return products
    .filter(p => p.stock > 0)
    .map(p => ({
      ...p,
      discountedPrice: calculateDiscount(p),
      shipping: calculateShipping(p),
      taxes: calculateTaxes(p),
    }))
    .sort((a, b) => b.popularity - a.popularity);
}

function ProductList() {
  const products = useProducts();
  return <div>{/* render */}</div>;
}

Category 6: Accessibility (Medium Priority)

Red Flag: Missing Semantic HTML

// ❌ QUESTION: Not accessible
function Button({ onClick, label }) {
  return (
    <div onClick={onClick} className="button">
      {label}
    </div>
  );
}

// ✅ APPROVE: Proper semantics
function Button({ onClick, label }) {
  return <button onClick={onClick}>{label}</button>;
}

Red Flag: Missing ARIA Labels

// ❌ QUESTION: Screen reader won't know what this is
<button onClick={handleClose}>
  <X /> {/* Icon only */}
</button>

// ✅ APPROVE: Accessible
<button onClick={handleClose} aria-label="Close dialog">
  <X />
</button>

// ✅ ALSO GOOD: Visible text
<button onClick={handleClose}>
  <X />
  <span>Close</span>
</button>

Red Flag: Keyboard Navigation Issues

// ❌ QUESTION: Can't navigate with keyboard
<div onClick={handleClick}>Click me</div>

// ✅ APPROVE: Keyboard accessible
<div
  role="button"
  tabIndex={0}
  onClick={handleClick}
  onKeyDown={(e) => {
    if (e.key === 'Enter' || e.key === ' ') {
      handleClick();
    }
  }}
>
  Click me
</div>

// ✅ BETTER: Use button element
<button onClick={handleClick}>Click me</button>

Category 7: Error Handling (Medium Priority)

Red Flag: Unhandled Promise Rejections

// ❌ QUESTION: What if this fails?
useEffect(() => {
  fetchData().then(setData);
}, []);

// ✅ APPROVE: Error handling
useEffect(() => {
  fetchData()
    .then(setData)
    .catch(error => {
      console.error('Failed to fetch data:', error);
      setError(error);
    });
}, []);

// ✅ BETTER: Async/await with try-catch
useEffect(() => {
  async function loadData() {
    try {
      const data = await fetchData();
      setData(data);
    } catch (error) {
      console.error('Failed to fetch data:', error);
      setError(error);
    }
  }

  loadData();
}, []);

Red Flag: Silent Failures

// ❌ BLOCK: Errors disappear
try {
  await updateUser(userId, data);
} catch (error) {
  // Nothing happens - user thinks it worked!
}

// ✅ APPROVE: Show error to user
try {
  await updateUser(userId, data);
  toast.success('Profile updated');
} catch (error) {
  toast.error('Failed to update profile');
  console.error(error);
}

Category 8: Testing (Medium Priority)

Red Flag: Untestable Code

// ❌ QUESTION: Hard to test - API call inside component
function UserProfile({ userId }: { userId: string }) {
  const [user, setUser] = useState(null);

  useEffect(() => {
    fetch(`/api/users/${userId}`)
      .then(res => res.json())
      .then(setUser);
  }, [userId]);

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

// ✅ APPROVE: Extract hook - easy to test
function useUser(userId: string) {
  const [user, setUser] = useState(null);

  useEffect(() => {
    fetch(`/api/users/${userId}`)
      .then(res => res.json())
      .then(setUser);
  }, [userId]);

  return user;
}

function UserProfile({ userId }: { userId: string }) {
  const user = useUser(userId);
  return <div>{user?.name}</div>;
}

// Test the hook in isolation
// Test the component with mocked hook

Red Flag: Missing Critical Tests

// Component that handles payments
function CheckoutForm() {
  const handleSubmit = async (data) => {
    await processPayment(data);
    await createOrder(data);
    await sendConfirmation(data);
  };

  return <form onSubmit={handleSubmit}>...</form>;
}

// ❌ QUESTION: Where are the tests?
// No tests for:
// - Payment processing
// - Error handling
// - Edge cases

Review comment:

⚠️ This component handles payment processing but I don't see tests. Could we add:

1. Test successful payment flow
2. Test payment failure handling
3. Test validation errors
4. Test edge cases (e.g., network timeout)

Payment code should have high test coverage.

What NOT to Comment On

❌ Don't Nitpick Style (Use Prettier/ESLint)

// ❌ Bad review comment:
"Please use single quotes instead of double quotes"
"Add a blank line here"
"This line is too long"

// ✅ Better: Configure Prettier/ESLint
// Let tools handle formatting
// Save reviews for meaningful feedback

❌ Don't Enforce Personal Preferences

// ❌ Bad review comment:
"I prefer to call this 'handleButtonClick' instead of 'onClick'"
"Can you use a for loop instead of map?"
"I don't like ternaries, use if/else"

// ✅ Better: Only comment if there's a real issue
// "This variable name is misleading because..."
// "This algorithm is O(n²), could we use..."

❌ Don't Rewrite Code in Comments

// ❌ Bad review comment:
"Here's how I would write this:
function myBetterVersion() {
  // 50 lines of alternative implementation
}
"

// ✅ Better:
"This could be simplified. What do you think about using 
Array.reduce here instead of multiple loops?"

The Art of Constructive Feedback

Use Questions, Not Commands

// ❌ Aggressive
"This is wrong. Use useMemo."

// ✅ Constructive
"Could we use useMemo here? This calculation runs on every render 
and might cause performance issues with large datasets."

Explain the "Why"

// ❌ Not helpful
"Add error handling."

// ✅ Helpful
"Could we add error handling here? If the API fails, users won't 
know what happened. A toast notification would improve UX."

Offer Alternatives

// ❌ Vague
"This could be better."

// ✅ Specific
"Two options for this:
1. Extract to custom hook for reusability
2. Use React Query for caching

Option 1 if we use this once, option 2 if we need it in multiple places."

Distinguish Blocking vs Non-Blocking

// 🔴 BLOCKING (must fix):
"⚠️ BLOCKING: This has an XSS vulnerability..."

// 🟡 NON-BLOCKING (suggestion):
"💡 Suggestion: This could be optimized with useMemo..."

// 🟢 PRAISE (reinforce good practices):
"✅ Nice! Good error handling here."

The Review Checklist

Quick Scan (2 minutes)

[ ] Does it build?
[ ] Are there any any types?
[ ] Any obvious security issues?
[ ] Tests included?

Deep Review (10-20 minutes)

Type Safety:

[ ] No liberal any usage
[ ] Type assertions justified
[ ] Null checks present
[ ] Proper type narrowing

React Patterns:

[ ] Hooks rules followed
[ ] Dependencies correct
[ ] No unnecessary re-renders
[ ] No state mutation

Performance:

[ ] No expensive operations in render
[ ] Bundle size considered
[ ] List keys proper

Security:

[ ] No XSS vulnerabilities
[ ] No exposed secrets
[ ] Input sanitized

Architecture:

[ ] Components focused
[ ] Logic extracted to hooks
[ ] No prop drilling

Accessibility:

[ ] Semantic HTML
[ ] ARIA labels present
[ ] Keyboard navigation works

Error Handling:

[ ] Promises catch errors
[ ] User sees errors
[ ] Graceful degradation

Final Check

[ ] Would I be comfortable merging this?
[ ] Are my comments constructive?
[ ] Did I explain why, not just what?

Example Review Comments

Excellent Review Comment

⚠️ Type Safety Issue

Using `any` here removes type safety. If the API shape changes, 
we won't catch it until runtime.

Could we define an interface?

interface UserResponse {
  id: string;
  name: string;
  email: string;
}

Then validate with Zod:
const UserSchema = z.object({...});
const user = UserSchema.parse(response);

This catches API changes at the boundary.

Poor Review Comment

This is wrong. Use proper types.

When to Approve vs Request Changes

Approve ✅

Minor suggestions only
No blocking issues
Good code that teaches you something
Meets team standards

Request Changes 🔄

Type safety violations
Security issues
Performance problems
Breaking changes without discussion
Missing critical tests

Comment Only 💬

Suggestions for improvement
Questions for clarity
Praise for good patterns
Learning opportunities

Conclusion

Good code review is about:

Catching bugs before they reach users
Teaching through constructive feedback
Maintaining standards without nitpicking
Building trust with respectful comments

Bad code review is:

Nitpicking style
Enforcing preferences
Being vague
Rewriting everything

The golden rule: Review code like you'd want yours reviewed. Be thorough on what matters. Be kind on everything else.

Focus on:

✅ Type safety
✅ React patterns
✅ Performance
✅ Security
✅ Architecture

Ignore:

❌ Style preferences
❌ Personal taste
❌ Bikeshedding

Your job isn't to make code look like you wrote it. It's to make code better than it was.

What do you look for in code reviews? Share your best review practices!

Props vs Hooks vs HOCs

Tarun Moorjani — Fri, 06 Feb 2026 16:27:24 +0000

Render Props vs Hooks vs HOCs: The TypeScript Perspective

Tarun Moorjani ・ Feb 6

#webdev #react #typescript #programming

Render Props vs Hooks vs HOCs: The TypeScript Perspective

Tarun Moorjani — Fri, 06 Feb 2026 16:26:08 +0000

I've refactored the same piece of logic three different ways in my career:

2016: Higher-Order Components everywhere

export default withAuth(withTheme(withRouter(MyComponent)));

2018: Render Props for everything

<Auth>
  {({ user }) => (
    <Theme>
      {({ theme }) => (
        <Router>
          {({ navigate }) => (
            <MyComponent user={user} theme={theme} navigate={navigate} />
          )}
        </Router>
      )}
    </Theme>
  )}
</Auth>

2020: Hooks have won

const user = useAuth();
const theme = useTheme();
const navigate = useNavigate();
return <MyComponent user={user} theme={theme} navigate={navigate} />;

Each pattern emerged to solve real problems. Each has trade-offs. And TypeScript changes the equation significantly.

Let me show you which pattern to use when, and how TypeScript makes (or breaks) each approach.

The Evolution: Why We Have Three Patterns

Higher-Order Components (2015-2018)

Problem they solved: Component logic reuse before hooks
Pattern: Wrap components to inject props
Peak usage: React 15-16 era

Render Props (2017-2019)

Problem they solved: HOC limitations (wrapper hell, prop naming collisions)
Pattern: Pass render functions as props
Peak usage: React 16 era

Hooks (2019-Present)

Problem they solved: Both HOC and Render Props limitations
Pattern: Compose logic with functions, not components
Peak usage: React 16.8+

Pattern 1: Higher-Order Components (HOCs)

Basic HOC with TypeScript

// withAuth.tsx
import { ComponentType } from 'react';

interface User {
  id: string;
  name: string;
  email: string;
}

interface WithAuthProps {
  user: User;
  logout: () => void;
}

// ✅ Type-safe HOC
export function withAuth<P extends object>(
  Component: ComponentType<P & WithAuthProps>
): ComponentType<P> {
  return function AuthenticatedComponent(props: P) {
    const user = useAuth();
    const logout = useLogout();

    if (!user) {
      return <div>Please log in</div>;
    }

    return <Component {...props} user={user} logout={logout} />;
  };
}

// Usage
interface DashboardProps {
  title: string;
}

function Dashboard({ title, user, logout }: DashboardProps & WithAuthProps) {
  return (
    <div>
      <h1>{title}</h1>
      <p>Welcome, {user.name}</p>
      <button onClick={logout}>Logout</button>
    </div>
  );
}

export default withAuth(Dashboard);

// TypeScript knows Dashboard needs 'title' prop
// TypeScript knows 'user' and 'logout' are injected
<Dashboard title="My Dashboard" />

Advanced: Generic HOC

// withData.tsx
interface WithDataProps<T> {
  data: T;
  isLoading: boolean;
  error: Error | null;
}

export function withData<T, P extends object>(
  fetchData: () => Promise<T>
) {
  return function (
    Component: ComponentType<P & WithDataProps<T>>
  ): ComponentType<P> {
    return function DataComponent(props: P) {
      const [data, setData] = useState<T | null>(null);
      const [isLoading, setIsLoading] = useState(true);
      const [error, setError] = useState<Error | null>(null);

      useEffect(() => {
        fetchData()
          .then(setData)
          .catch(setError)
          .finally(() => setIsLoading(false));
      }, []);

      if (isLoading) return <div>Loading...</div>;
      if (error) return <div>Error: {error.message}</div>;
      if (!data) return null;

      return <Component {...props} data={data} isLoading={false} error={null} />;
    };
  };
}

// Usage - full type inference!
interface User {
  id: string;
  name: string;
}

interface UserListProps {
  filter: string;
}

function UserList({ 
  filter, 
  data, 
  isLoading 
}: UserListProps & WithDataProps<User[]>) {
  const filtered = data.filter(user => 
    user.name.includes(filter)
  );

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

export default withData<User[]>(fetchUsers)(UserList);

Composing Multiple HOCs

// The infamous wrapper hell
export default withAuth(
  withTheme(
    withRouter(
      withData(fetchUsers)(
        UserList
      )
    )
  )
);

// Better: compose utility
function compose<P extends object>(
  ...hocs: Array<(component: ComponentType<any>) => ComponentType<any>>
) {
  return (component: ComponentType<P>) =>
    hocs.reduceRight(
      (acc, hoc) => hoc(acc),
      component
    );
}

// Much cleaner
export default compose(
  withAuth,
  withTheme,
  withRouter,
  withData(fetchUsers)
)(UserList);

HOC Pros and Cons

Pros:

✅ Logic encapsulation
✅ Can modify props
✅ Works with class components
✅ Clear dependency chain

Cons with TypeScript:

❌ Type inference can break with multiple HOCs
❌ Wrapper hell in DevTools
❌ Prop naming collisions
❌ Difficult to trace where props come from
❌ Can't use inside component body

When to use HOCs:

Legacy codebases with class components
When you need to modify component props
When composing third-party libraries
When the pattern is already established

Pattern 2: Render Props

Basic Render Props with TypeScript

// Auth.tsx
interface User {
  id: string;
  name: string;
  email: string;
}

interface AuthRenderProps {
  user: User | null;
  login: (email: string, password: string) => Promise<void>;
  logout: () => void;
  isLoading: boolean;
}

interface AuthProps {
  children: (props: AuthRenderProps) => React.ReactNode;
}

export function Auth({ children }: AuthProps) {
  const [user, setUser] = useState<User | null>(null);
  const [isLoading, setIsLoading] = useState(true);

  const login = async (email: string, password: string) => {
    const user = await loginAPI(email, password);
    setUser(user);
  };

  const logout = () => {
    setUser(null);
  };

  useEffect(() => {
    // Check for existing session
    checkSession().then(setUser).finally(() => setIsLoading(false));
  }, []);

  return children({ user, login, logout, isLoading });
}

// Usage - full type safety!
function App() {
  return (
    <Auth>
      {({ user, login, logout, isLoading }) => {
        if (isLoading) return <div>Loading...</div>;

        if (!user) {
          return <LoginForm onLogin={login} />;
        }

        return (
          <div>
            <h1>Welcome, {user.name}</h1>
            <button onClick={logout}>Logout</button>
          </div>
        );
      }}
    </Auth>
  );
}

Generic Render Props

// DataFetcher.tsx
interface DataFetcherRenderProps<T> {
  data: T | null;
  isLoading: boolean;
  error: Error | null;
  refetch: () => void;
}

interface DataFetcherProps<T> {
  fetchData: () => Promise<T>;
  children: (props: DataFetcherRenderProps<T>) => React.ReactNode;
}

export function DataFetcher<T>({ 
  fetchData, 
  children 
}: DataFetcherProps<T>) {
  const [data, setData] = useState<T | null>(null);
  const [isLoading, setIsLoading] = useState(true);
  const [error, setError] = useState<Error | null>(null);

  const fetch = useCallback(() => {
    setIsLoading(true);
    setError(null);
    fetchData()
      .then(setData)
      .catch(setError)
      .finally(() => setIsLoading(false));
  }, [fetchData]);

  useEffect(() => {
    fetch();
  }, [fetch]);

  return children({ 
    data, 
    isLoading, 
    error, 
    refetch: fetch 
  });
}

// Usage - TypeScript infers T from fetchUsers
interface User {
  id: string;
  name: string;
}

function UserList() {
  return (
    <DataFetcher fetchData={fetchUsers}>
      {({ data, isLoading, error, refetch }) => {
        if (isLoading) return <div>Loading...</div>;
        if (error) return <div>Error: {error.message}</div>;
        if (!data) return null;

        // TypeScript knows data is User[]
        return (
          <div>
            <button onClick={refetch}>Refresh</button>
            <ul>
              {data.map(user => (
                <li key={user.id}>{user.name}</li>
              ))}
            </ul>
          </div>
        );
      }}
    </DataFetcher>
  );
}

Render Props with Default Rendering

// Toggle.tsx
interface ToggleRenderProps {
  isOn: boolean;
  toggle: () => void;
  setOn: (value: boolean) => void;
}

interface ToggleProps {
  defaultOn?: boolean;
  children?: (props: ToggleRenderProps) => React.ReactNode;
  // Optional: provide default UI
  render?: (props: ToggleRenderProps) => React.ReactNode;
}

export function Toggle({ 
  defaultOn = false, 
  children, 
  render 
}: ToggleProps) {
  const [isOn, setIsOn] = useState(defaultOn);

  const toggle = () => setIsOn(prev => !prev);
  const setOn = (value: boolean) => setIsOn(value);

  const renderProps: ToggleRenderProps = { isOn, toggle, setOn };

  // Use render prop if provided, otherwise children, otherwise default
  if (render) return render(renderProps);
  if (children) return children(renderProps);

  // Default UI
  return (
    <button onClick={toggle}>
      {isOn ? 'ON' : 'OFF'}
    </button>
  );
}

// Usage - multiple ways
// 1. Default UI
<Toggle defaultOn={true} />

// 2. Children render prop
<Toggle>
  {({ isOn, toggle }) => (
    <label>
      <input type="checkbox" checked={isOn} onChange={toggle} />
      {isOn ? 'Enabled' : 'Disabled'}
    </label>
  )}
</Toggle>

// 3. Render prop
<Toggle
  render={({ isOn, toggle }) => (
    <div className={isOn ? 'active' : 'inactive'} onClick={toggle}>
      Click me
    </div>
  )}
/>

Render Props Composition (The Pain)

// ❌ Callback hell
function App() {
  return (
    <Auth>
      {({ user }) => (
        <Theme>
          {({ theme }) => (
            <DataFetcher fetchData={fetchPosts}>
              {({ data: posts }) => (
                <WindowSize>
                  {({ width }) => (
                    <Dashboard 
                      user={user} 
                      theme={theme} 
                      posts={posts} 
                      width={width} 
                    />
                  )}
                </WindowSize>
              )}
            </DataFetcher>
          )}
        </Theme>
      )}
    </Auth>
  );
}

Render Props Pros and Cons

Pros:

✅ Explicit data flow
✅ No prop naming collisions
✅ Can customize rendering completely
✅ Good TypeScript inference

Cons:

❌ Callback hell with multiple render props
❌ Harder to test
❌ Extra components in tree
❌ Can't use inside component body
❌ Performance overhead (function creation)

When to use Render Props:

When you need complete rendering control
When building headless UI components
When the consumer needs to decide layout
Legacy codebases using this pattern

Pattern 3: Hooks (The Winner)

Basic Hook

// useAuth.ts
interface User {
  id: string;
  name: string;
  email: string;
}

interface UseAuthReturn {
  user: User | null;
  login: (email: string, password: string) => Promise<void>;
  logout: () => void;
  isLoading: boolean;
}

export function useAuth(): UseAuthReturn {
  const [user, setUser] = useState<User | null>(null);
  const [isLoading, setIsLoading] = useState(true);

  const login = useCallback(async (email: string, password: string) => {
    const user = await loginAPI(email, password);
    setUser(user);
  }, []);

  const logout = useCallback(() => {
    setUser(null);
  }, []);

  useEffect(() => {
    checkSession()
      .then(setUser)
      .finally(() => setIsLoading(false));
  }, []);

  return { user, login, logout, isLoading };
}

// Usage - clean and simple!
function App() {
  const { user, login, logout, isLoading } = useAuth();

  if (isLoading) return <div>Loading...</div>;

  if (!user) {
    return <LoginForm onLogin={login} />;
  }

  return (
    <div>
      <h1>Welcome, {user.name}</h1>
      <button onClick={logout}>Logout</button>
    </div>
  );
}

Generic Hook

// useData.ts
interface UseDataOptions {
  autoFetch?: boolean;
}

interface UseDataReturn<T> {
  data: T | null;
  isLoading: boolean;
  error: Error | null;
  refetch: () => void;
}

export function useData<T>(
  fetchData: () => Promise<T>,
  options: UseDataOptions = {}
): UseDataReturn<T> {
  const { autoFetch = true } = options;
  const [data, setData] = useState<T | null>(null);
  const [isLoading, setIsLoading] = useState(autoFetch);
  const [error, setError] = useState<Error | null>(null);

  const fetch = useCallback(() => {
    setIsLoading(true);
    setError(null);
    fetchData()
      .then(setData)
      .catch(setError)
      .finally(() => setIsLoading(false));
  }, [fetchData]);

  useEffect(() => {
    if (autoFetch) {
      fetch();
    }
  }, [autoFetch, fetch]);

  return { data, isLoading, error, refetch: fetch };
}

// Usage - beautiful type inference
interface User {
  id: string;
  name: string;
}

function UserList() {
  const { data, isLoading, error, refetch } = useData(fetchUsers);

  if (isLoading) return <div>Loading...</div>;
  if (error) return <div>Error: {error.message}</div>;
  if (!data) return null;

  // TypeScript knows data is User[]
  return (
    <div>
      <button onClick={refetch}>Refresh</button>
      <ul>
        {data.map(user => (
          <li key={user.id}>{user.name}</li>
        ))}
      </ul>
    </div>
  );
}

Composing Hooks (The Dream)

// No nesting, no wrappers, just composition!
function Dashboard() {
  const { user } = useAuth();
  const { theme } = useTheme();
  const { data: posts } = useData(fetchPosts);
  const { width } = useWindowSize();

  return (
    <div className={theme}>
      <h1>Welcome, {user.name}</h1>
      <div>Window width: {width}px</div>
      <PostList posts={posts} />
    </div>
  );
}

Hooks Pros and Cons

Pros:

✅ No wrapper components
✅ Perfect composition
✅ Excellent TypeScript inference
✅ Testable in isolation
✅ Can use conditionally (with rules)
✅ Smaller bundle size
✅ Better DevTools experience

Cons:

❌ Rules of Hooks (can't use in loops/conditions)
❌ Can't use in class components
❌ Learning curve for beginners
❌ Requires React 16.8+

When to use Hooks:

Always, if you can
New features and components
Any modern React codebase

Side-by-Side Comparison

The Same Logic: Three Ways

Scenario: Fetch and display user data with loading and error states.

// 1. HOC Version
interface WithUserDataProps {
  userData: User[];
  isLoading: boolean;
  error: Error | null;
}

function withUserData<P extends object>(
  Component: ComponentType<P & WithUserDataProps>
): ComponentType<P> {
  return function UserDataComponent(props: P) {
    const [userData, setUserData] = useState<User[]>([]);
    const [isLoading, setIsLoading] = useState(true);
    const [error, setError] = useState<Error | null>(null);

    useEffect(() => {
      fetchUsers()
        .then(setUserData)
        .catch(setError)
        .finally(() => setIsLoading(false));
    }, []);

    return (
      <Component 
        {...props} 
        userData={userData} 
        isLoading={isLoading} 
        error={error} 
      />
    );
  };
}

// Usage
function UserList({ userData, isLoading, error }: WithUserDataProps) {
  if (isLoading) return <div>Loading...</div>;
  if (error) return <div>Error: {error.message}</div>;
  return <ul>{userData.map(u => <li key={u.id}>{u.name}</li>)}</ul>;
}

export default withUserData(UserList);


// 2. Render Props Version
interface UserDataRenderProps {
  userData: User[];
  isLoading: boolean;
  error: Error | null;
}

function UserDataProvider({ 
  children 
}: { 
  children: (props: UserDataRenderProps) => React.ReactNode 
}) {
  const [userData, setUserData] = useState<User[]>([]);
  const [isLoading, setIsLoading] = useState(true);
  const [error, setError] = useState<Error | null>(null);

  useEffect(() => {
    fetchUsers()
      .then(setUserData)
      .catch(setError)
      .finally(() => setIsLoading(false));
  }, []);

  return children({ userData, isLoading, error });
}

// Usage
function UserList() {
  return (
    <UserDataProvider>
      {({ userData, isLoading, error }) => {
        if (isLoading) return <div>Loading...</div>;
        if (error) return <div>Error: {error.message}</div>;
        return <ul>{userData.map(u => <li key={u.id}>{u.name}</li>)}</ul>;
      }}
    </UserDataProvider>
  );
}


// 3. Hook Version
interface UseUserDataReturn {
  userData: User[];
  isLoading: boolean;
  error: Error | null;
}

function useUserData(): UseUserDataReturn {
  const [userData, setUserData] = useState<User[]>([]);
  const [isLoading, setIsLoading] = useState(true);
  const [error, setError] = useState<Error | null>(null);

  useEffect(() => {
    fetchUsers()
      .then(setUserData)
      .catch(setError)
      .finally(() => setIsLoading(false));
  }, []);

  return { userData, isLoading, error };
}

// Usage
function UserList() {
  const { userData, isLoading, error } = useUserData();

  if (isLoading) return <div>Loading...</div>;
  if (error) return <div>Error: {error.message}</div>;
  return <ul>{userData.map(u => <li key={u.id}>{u.name}</li>)}</ul>;
}

TypeScript Comparison

Feature	HOC	Render Props	Hooks
Type Inference	🟡 Breaks with nesting	🟢 Excellent	🟢 Excellent
Prop Typing	🔴 Complex generics	🟢 Straightforward	🟢 Straightforward
Return Type	🟡 Needs explicit typing	🟢 Inferred	🟢 Inferred
Generic Support	🟡 Complex	🟢 Good	🟢 Excellent
Ref Forwarding	🔴 Difficult	N/A	🟢 Easy

Migration Strategies

HOC to Hook

// Before: HOC
interface WithAuthProps {
  user: User;
  logout: () => void;
}

function withAuth<P extends object>(
  Component: ComponentType<P & WithAuthProps>
) {
  return function AuthComponent(props: P) {
    const user = useAuth();
    const logout = useLogout();
    return <Component {...props} user={user} logout={logout} />;
  };
}

export default withAuth(Dashboard);

// After: Hook
function Dashboard() {
  const user = useAuth();
  const logout = useLogout();

  // Component logic
}

Render Props to Hook

// Before: Render Props
<DataFetcher fetchData={fetchUsers}>
  {({ data, isLoading, error }) => (
    <UserList data={data} isLoading={isLoading} error={error} />
  )}
</DataFetcher>

// After: Hook
function UserList() {
  const { data, isLoading, error } = useData(fetchUsers);
  // Component logic
}

Gradual Migration Strategy

// Step 1: Keep HOC/Render Prop, extract logic to hook
function useUserData() {
  // Logic extracted here
}

function withUserData<P>(Component: ComponentType<P & WithUserDataProps>) {
  return function(props: P) {
    const userData = useUserData(); // Use the hook internally
    return <Component {...props} {...userData} />;
  };
}

// Step 2: Migrate components to use hook directly
// Step 3: Remove HOC once all components migrated

Hybrid Approaches

Hook + Render Prop (Headless UI Pattern)

// Provide both APIs!
interface SelectRenderProps<T> {
  value: T | null;
  isOpen: boolean;
  toggle: () => void;
  select: (value: T) => void;
}

interface SelectProps<T> {
  value: T | null;
  onChange: (value: T) => void;
  children: (props: SelectRenderProps<T>) => React.ReactNode;
}

// Hook for internal logic
export function useSelect<T>(
  value: T | null,
  onChange: (value: T) => void
) {
  const [isOpen, setIsOpen] = useState(false);

  const toggle = () => setIsOpen(prev => !prev);
  const select = (value: T) => {
    onChange(value);
    setIsOpen(false);
  };

  return { value, isOpen, toggle, select };
}

// Component using render prop
export function Select<T>({ value, onChange, children }: SelectProps<T>) {
  const selectProps = useSelect(value, onChange);
  return children(selectProps);
}

// Usage - consumers choose!

// 1. Use the component with render prop
<Select value={value} onChange={setValue}>
  {({ isOpen, toggle, select }) => (
    <div>
      <button onClick={toggle}>Select</button>
      {isOpen && <Options onSelect={select} />}
    </div>
  )}
</Select>

// 2. Or use the hook directly
function CustomSelect() {
  const { isOpen, toggle, select } = useSelect(value, setValue);
  return (
    <div>
      <button onClick={toggle}>Select</button>
      {isOpen && <Options onSelect={select} />}
    </div>
  );
}

Hook + HOC for Legacy Support

// Modern hook
export function useFeatureFlags() {
  const [flags, setFlags] = useState<Record<string, boolean>>({});

  useEffect(() => {
    fetchFeatureFlags().then(setFlags);
  }, []);

  return flags;
}

// Legacy HOC wrapper for class components
export function withFeatureFlags<P extends object>(
  Component: ComponentType<P & { flags: Record<string, boolean> }>
): ComponentType<P> {
  return function FeatureFlagsComponent(props: P) {
    const flags = useFeatureFlags();
    return <Component {...props} flags={flags} />;
  };
}

// Modern components use hook
function ModernComponent() {
  const flags = useFeatureFlags();
  // ...
}

// Legacy class components use HOC
class LegacyComponent extends React.Component<{ flags: Record<string, boolean> }> {
  render() {
    const { flags } = this.props;
    // ...
  }
}

export default withFeatureFlags(LegacyComponent);

Real-World Decision Tree

Do you have class components?
├─ Yes → HOC or Render Props
└─ No → Hooks

Do you need to modify component props?
├─ Yes → HOC
└─ No → Hooks or Render Props

Do consumers need rendering control?
├─ Yes → Render Props or Hooks + Render Props
└─ No → Hooks

Is this a new feature?
├─ Yes → Hooks
└─ No → Match existing pattern

Do you need to compose multiple concerns?
├─ Yes → Hooks (much cleaner)
└─ No → Any pattern works

Are you building a library?
├─ Yes → Hooks + Render Props (both APIs)
└─ No → Hooks

Performance Comparison

Bundle Size Impact

// HOC - adds wrapper component
// Bundle: +500 bytes per HOC

// Render Props - adds component + render function
// Bundle: +600 bytes per render prop

// Hooks - just a function
// Bundle: +200 bytes per hook

Runtime Performance

// HOC
// - Creates wrapper components (extra tree depth)
// - Props spreading overhead
// - React.memo needed to prevent re-renders

// Render Props  
// - Creates components (extra tree depth)
// - Function creation on every render
// - Callback overhead

// Hooks
// - No extra components
// - Optimized by React
// - useMemo/useCallback when needed

Best Practices by Pattern

HOC Best Practices

// ✅ Forward refs
export function withAuth<P extends object>(
  Component: ComponentType<P>
) {
  return forwardRef<any, P>((props, ref) => {
    const auth = useAuth();
    return <Component ref={ref} {...props} {...auth} />;
  });
}

// ✅ Hoist non-react statics
import hoistNonReactStatics from 'hoist-non-react-statics';

export function withAuth<P>(Component: ComponentType<P>) {
  const WithAuth = (props: P) => {
    // ...
  };

  return hoistNonReactStatics(WithAuth, Component);
}

// ✅ Display names for DevTools
WithAuth.displayName = `WithAuth(${Component.displayName || Component.name})`;

Render Props Best Practices

// ✅ Memoize render functions
function App() {
  const renderUsers = useCallback(({ users }: { users: User[] }) => (
    <UserList users={users} />
  ), []);

  return <UserDataProvider>{renderUsers}</UserDataProvider>;
}

// ✅ Provide default rendering
interface ToggleProps {
  children?: (props: ToggleRenderProps) => ReactNode;
  render?: (props: ToggleRenderProps) => ReactNode;
}

export function Toggle({ children, render }: ToggleProps) {
  const props = useToggle();

  if (render) return render(props);
  if (children) return children(props);

  return <DefaultToggleUI {...props} />;
}

Hook Best Practices

// ✅ Return objects, not arrays (usually)
// Better
function useAuth() {
  return { user, login, logout };
}

// Unless mimicking useState
function useToggle(initialValue: boolean) {
  return [value, toggle] as const;
}

// ✅ Prefix with 'use'
function useWindowSize() { /* ... */ }

// ✅ Extract to custom hooks early
function MyComponent() {
  // ❌ Complex logic in component
  const [users, setUsers] = useState([]);
  useEffect(() => { /* ... */ }, []);

  // ✅ Extract to hook
  const users = useUsers();
}

Conclusion

The composition pattern landscape has evolved:

HOCs (2015-2018):

Legacy pattern, still useful for class components
TypeScript support is complex
Use when you must, not by default

Render Props (2017-2019):

Transitional pattern
Good TypeScript support
Use for headless UI libraries
Avoid in new applications

Hooks (2019-Present):

Modern standard
Excellent TypeScript support
Use for everything new
Migrate to hooks when refactoring

The decision matrix:

New code: Hooks
Class components: HOC
Headless UI: Hooks + Render Props
Library API: Hooks + optional Render Props
Legacy: Match existing pattern

TypeScript makes hooks even better than in JavaScript. Type inference works perfectly, composition is clean, and there's no wrapper hell.

If you're starting fresh: use hooks. If you're maintaining legacy code: migrate to hooks. The era of HOCs and Render Props is over.

Hooks won.

Which pattern do you use most? Share your migration stories in the comments!

Code Splitting TypeScript React: Lazy Loading Done Right - Learn how lazy loading can help you

Tarun Moorjani — Tue, 20 Jan 2026 19:28:21 +0000

Code Splitting TypeScript React: Lazy Loading Done Right

Tarun Moorjani ・ Jan 20

#webdev #typescript #react #discuss

Code Splitting TypeScript React: Lazy Loading Done Right

Tarun Moorjani — Tue, 20 Jan 2026 19:26:29 +0000

My React app was 2.5MB. Users on slow connections waited 15 seconds to see anything. Time to Interactive was abysmal. Google Lighthouse scores were red across the board.

I knew I needed code splitting. I'd heard about React.lazy() and dynamic imports. But every tutorial showed JavaScript examples, and when I tried to add TypeScript, nothing worked:

// ❌ This breaks everything
const Dashboard = React.lazy(() => import('./Dashboard'));
// Type error: Cannot find module './Dashboard' or its corresponding type declarations

The types didn't import correctly. Suspense boundaries felt clunky. Preloading was a mystery. And I had no idea if my splitting strategy actually worked.

After months of optimization, I got my initial bundle down to 180KB. Time to Interactive dropped from 15 seconds to 2 seconds. Here's everything I learned about code splitting with TypeScript.

The Fundamentals: How Code Splitting Works

Before diving into TypeScript, understand what's happening:

Without code splitting:

app.js (2.5MB) → User waits for entire app before seeing anything

With code splitting:

main.js (180KB)    → User sees app immediately
dashboard.js (400KB) → Loads when user navigates to dashboard
admin.js (600KB)    → Loads only for admin users
charts.js (350KB)   → Loads when user opens charts

Code splitting transforms one massive bundle into smaller chunks that load on-demand.

Pattern 1: Basic Type-Safe Lazy Loading

The Problem with Simple Lazy Loading

// ❌ Type inference breaks
const Dashboard = React.lazy(() => import('./Dashboard'));

// Later...
<Dashboard />  // No prop autocomplete, no type checking

The Solution: Proper Component Types

// Dashboard.tsx
interface DashboardProps {
  userId: string;
  onLogout: () => void;
}

export default function Dashboard({ userId, onLogout }: DashboardProps) {
  return <div>Dashboard for {userId}</div>;
}

// App.tsx
import { lazy, ComponentType } from 'react';

// ✅ Explicitly type the lazy component
const Dashboard = lazy<ComponentType<DashboardProps>>(
  () => import('./Dashboard')
);

// Usage - full type safety!
function App() {
  return (
    <Suspense fallback={<Loading />}>
      <Dashboard 
        userId="123" 
        onLogout={handleLogout}
        // TypeScript knows the props!
      />
    </Suspense>
  );
}

Generic Lazy Loader Helper

// utils/lazy.ts
import { lazy, ComponentType } from 'react';

/**
 * Type-safe wrapper around React.lazy
 */
export function lazyLoad<T extends ComponentType<any>>(
  factory: () => Promise<{ default: T }>
): T {
  return lazy(factory) as unknown as T;
}

// Usage
const Dashboard = lazyLoad<ComponentType<DashboardProps>>(
  () => import('./Dashboard')
);

// Even cleaner - infer from import
const Settings = lazyLoad(() => import('./Settings'));
// TypeScript infers the component type!

Pattern 2: Route-Based Code Splitting

Split by routes for maximum impact.

Basic Route Splitting

// routes.tsx
import { lazy } from 'react';
import { RouteObject } from 'react-router-dom';

// Lazy load route components
const HomePage = lazy(() => import('./pages/Home'));
const DashboardPage = lazy(() => import('./pages/Dashboard'));
const ProfilePage = lazy(() => import('./pages/Profile'));
const SettingsPage = lazy(() => import('./pages/Settings'));

export const routes: RouteObject[] = [
  {
    path: '/',
    element: <HomePage />,
  },
  {
    path: '/dashboard',
    element: <DashboardPage />,
  },
  {
    path: '/profile',
    element: <ProfilePage />,
  },
  {
    path: '/settings',
    element: <SettingsPage />,
  },
];

Suspense Wrapper for Routes

// components/RouteWithSuspense.tsx
import { Suspense, ComponentType } from 'react';

interface RouteWithSuspenseProps {
  component: ComponentType;
  fallback?: React.ReactNode;
}

export function RouteWithSuspense({ 
  component: Component, 
  fallback = <PageLoader /> 
}: RouteWithSuspenseProps) {
  return (
    <Suspense fallback={fallback}>
      <Component />
    </Suspense>
  );
}

// Usage
const routes: RouteObject[] = [
  {
    path: '/dashboard',
    element: <RouteWithSuspense component={DashboardPage} />,
  },
];

Nested Route Splitting

// App.tsx
import { Outlet } from 'react-router-dom';

const AdminLayout = lazy(() => import('./layouts/AdminLayout'));
const UserManagement = lazy(() => import('./pages/admin/UserManagement'));
const Analytics = lazy(() => import('./pages/admin/Analytics'));
const Reports = lazy(() => import('./pages/admin/Reports'));

export const routes: RouteObject[] = [
  {
    path: '/admin',
    element: (
      <Suspense fallback={<LayoutSkeleton />}>
        <AdminLayout />
      </Suspense>
    ),
    children: [
      {
        path: 'users',
        element: (
          <Suspense fallback={<PageSkeleton />}>
            <UserManagement />
          </Suspense>
        ),
      },
      {
        path: 'analytics',
        element: (
          <Suspense fallback={<PageSkeleton />}>
            <Analytics />
          </Suspense>
        ),
      },
      {
        path: 'reports',
        element: (
          <Suspense fallback={<PageSkeleton />}>
            <Reports />
          </Suspense>
        ),
      },
    ],
  },
];

Pattern 3: Component-Based Code Splitting

Split heavy components, not routes.

Identifying Split Candidates

// Heavy components to split:
// - Rich text editors
// - Chart libraries
// - Data tables
// - Modals (especially heavy ones)
// - Admin panels
// - Feature-flagged components

// Dashboard.tsx
import { lazy, useState } from 'react';

// ✅ Split heavy chart component
const ChartComponent = lazy(() => import('./ChartComponent'));

// ✅ Split heavy data table
const DataTable = lazy(() => import('./DataTable'));

export function Dashboard() {
  const [showChart, setShowChart] = useState(false);

  return (
    <div>
      <h1>Dashboard</h1>

      <button onClick={() => setShowChart(true)}>
        Show Chart
      </button>

      {showChart && (
        <Suspense fallback={<ChartSkeleton />}>
          <ChartComponent data={chartData} />
        </Suspense>
      )}

      <Suspense fallback={<TableSkeleton />}>
        <DataTable />
      </Suspense>
    </div>
  );
}

Modal Code Splitting

// Modals are perfect for code splitting - they're not needed until opened

interface UserModalProps {
  userId: string;
  onClose: () => void;
}

const UserModal = lazy(() => import('./modals/UserModal'));

function UserList() {
  const [selectedUserId, setSelectedUserId] = useState<string | null>(null);

  return (
    <div>
      {users.map(user => (
        <button key={user.id} onClick={() => setSelectedUserId(user.id)}>
          {user.name}
        </button>
      ))}

      {selectedUserId && (
        <Suspense fallback={<ModalSkeleton />}>
          <UserModal 
            userId={selectedUserId} 
            onClose={() => setSelectedUserId(null)} 
          />
        </Suspense>
      )}
    </div>
  );
}

Pattern 4: Library Code Splitting

Split large dependencies dynamically.

Dynamic Library Imports

// utils/dynamicImports.ts

/**
 * Lazy load Chart.js only when needed
 */
export async function loadChartLibrary() {
  const { Chart, registerables } = await import('chart.js');
  Chart.register(...registerables);
  return Chart;
}

/**
 * Lazy load date-fns only when needed
 */
export async function formatDate(date: Date, format: string) {
  const { format: formatFn } = await import('date-fns');
  return formatFn(date, format);
}

/**
 * Lazy load lodash functions
 */
export async function debounce<T extends (...args: any[]) => any>(
  func: T,
  wait: number
): Promise<T & { cancel: () => void }> {
  const { default: debounce } = await import('lodash/debounce');
  return debounce(func, wait);
}

// Usage
function ChartComponent() {
  const [Chart, setChart] = useState<typeof import('chart.js').Chart | null>(null);

  useEffect(() => {
    loadChartLibrary().then(setChart);
  }, []);

  if (!Chart) return <div>Loading chart...</div>;

  return <canvas ref={canvasRef} />;
}

Code Splitting Heavy Utilities

// utils/markdown.ts
/**
 * Don't import markdown library at app startup
 * Load it only when rendering markdown
 */
export async function renderMarkdown(markdown: string): Promise<string> {
  const { marked } = await import('marked');
  return marked(markdown);
}

// Usage
function MarkdownPreview({ content }: { content: string }) {
  const [html, setHtml] = useState<string>('');

  useEffect(() => {
    renderMarkdown(content).then(setHtml);
  }, [content]);

  return <div dangerouslySetInnerHTML={{ __html: html }} />;
}

Pattern 5: Preloading Strategies

Load code before it's needed.

Preload on Hover

// components/Link.tsx
import { Link as RouterLink, LinkProps } from 'react-router-dom';
import { ComponentType } from 'react';

interface PreloadableLinkProps extends LinkProps {
  preload?: () => Promise<{ default: ComponentType }>;
}

export function Link({ preload, ...props }: PreloadableLinkProps) {
  const handleMouseEnter = () => {
    if (preload) {
      // Start loading when user hovers
      preload();
    }
  };

  return (
    <RouterLink 
      {...props} 
      onMouseEnter={handleMouseEnter}
      onFocus={handleMouseEnter}
    />
  );
}

// Usage
<Link 
  to="/dashboard" 
  preload={() => import('./pages/Dashboard')}
>
  Go to Dashboard
</Link>

Preload on Idle

// utils/preload.ts
/**
 * Preload component during browser idle time
 */
export function preloadOnIdle(
  factory: () => Promise<{ default: ComponentType<any> }>
): void {
  if ('requestIdleCallback' in window) {
    requestIdleCallback(() => {
      factory();
    });
  } else {
    // Fallback for browsers without requestIdleCallback
    setTimeout(() => {
      factory();
    }, 1);
  }
}

// App.tsx
useEffect(() => {
  // Preload likely next pages during idle time
  preloadOnIdle(() => import('./pages/Dashboard'));
  preloadOnIdle(() => import('./pages/Profile'));
}, []);

Preload on Route Match

// hooks/usePreloadRoutes.ts
import { useLocation } from 'react-router-dom';
import { useEffect } from 'react';

interface RoutePreloadMap {
  [path: string]: () => Promise<{ default: ComponentType<any> }>;
}

const preloadMap: RoutePreloadMap = {
  '/': () => import('./pages/Dashboard'),
  '/profile': () => import('./pages/Settings'),
  '/dashboard': () => import('./pages/Analytics'),
};

export function usePreloadRoutes() {
  const location = useLocation();

  useEffect(() => {
    // Preload likely next route based on current route
    const nextRoute = preloadMap[location.pathname];
    if (nextRoute) {
      preloadOnIdle(nextRoute);
    }
  }, [location.pathname]);
}

Aggressive Preloading

// Preload everything after initial render
function App() {
  useEffect(() => {
    // Wait for initial render to complete
    setTimeout(() => {
      // Preload all routes
      import('./pages/Dashboard');
      import('./pages/Profile');
      import('./pages/Settings');
      import('./pages/Admin');
    }, 2000);
  }, []);

  return <Router />;
}

Pattern 6: Type-Safe Suspense Boundaries

Nested Suspense Boundaries

// Different loading states for different parts
function App() {
  return (
    <Suspense fallback={<AppShell />}>
      <Layout>
        <Suspense fallback={<Sidebar.Skeleton />}>
          <Sidebar />
        </Suspense>

        <Main>
          <Suspense fallback={<PageLoader />}>
            <Routes />
          </Suspense>
        </Main>
      </Layout>
    </Suspense>
  );
}

Error Boundaries with Suspense

// ErrorBoundary.tsx
import { Component, ReactNode } from 'react';

interface Props {
  fallback: (error: Error) => ReactNode;
  children: ReactNode;
}

interface State {
  error: Error | null;
}

export class ErrorBoundary extends Component<Props, State> {
  state: State = { error: null };

  static getDerivedStateFromError(error: Error): State {
    return { error };
  }

  render() {
    if (this.state.error) {
      return this.props.fallback(this.state.error);
    }

    return this.props.children;
  }
}

// Usage - wrap Suspense with ErrorBoundary
function App() {
  return (
    <ErrorBoundary fallback={(error) => <ErrorPage error={error} />}>
      <Suspense fallback={<Loading />}>
        <Dashboard />
      </Suspense>
    </ErrorBoundary>
  );
}

Custom Suspense Component

// components/SuspenseWithError.tsx
import { Suspense, ReactNode } from 'react';
import { ErrorBoundary } from './ErrorBoundary';

interface SuspenseWithErrorProps {
  children: ReactNode;
  fallback: ReactNode;
  errorFallback?: (error: Error) => ReactNode;
}

export function SuspenseWithError({
  children,
  fallback,
  errorFallback = (error) => <div>Error: {error.message}</div>,
}: SuspenseWithErrorProps) {
  return (
    <ErrorBoundary fallback={errorFallback}>
      <Suspense fallback={fallback}>
        {children}
      </Suspense>
    </ErrorBoundary>
  );
}

// Usage
<SuspenseWithError
  fallback={<PageLoader />}
  errorFallback={(error) => <ErrorPage error={error} />}
>
  <Dashboard />
</SuspenseWithError>

Pattern 7: Handling Shared Types

Type-Only Imports

// types/user.ts
export interface User {
  id: string;
  name: string;
  email: string;
}

// Dashboard.tsx
// ✅ Type-only import - doesn't affect bundle
import type { User } from '../types/user';

export default function Dashboard({ user }: { user: User }) {
  return <div>Hello {user.name}</div>;
}

Shared Type Definitions

// Create a types-only package for shared types
// This doesn't get bundled with code

// types/index.ts
export type { User } from './user';
export type { Post } from './post';
export type { Comment } from './comment';

// Components import types without bundling them
import type { User, Post } from '@/types';

Avoiding Type Duplication

// ❌ Bad - duplicates types
// Dashboard.tsx
interface User {
  id: string;
  name: string;
}

// Profile.tsx
interface User {
  id: string;
  name: string;
}

// ✅ Good - shared types
// types/user.ts
export interface User {
  id: string;
  name: string;
}

// Both import the same type
import type { User } from '@/types';

Pattern 8: Dynamic Import Utilities

Type-Safe Dynamic Component Loader

// utils/loadComponent.ts
import { ComponentType } from 'react';

interface LoadComponentOptions {
  fallback?: React.ReactNode;
  onError?: (error: Error) => void;
}

export function loadComponent<P = {}>(
  factory: () => Promise<{ default: ComponentType<P> }>,
  options: LoadComponentOptions = {}
): ComponentType<P> {
  const LazyComponent = React.lazy(factory);

  return function LoadedComponent(props: P) {
    return (
      <ErrorBoundary
        fallback={(error) => {
          options.onError?.(error);
          return <div>Failed to load component</div>;
        }}
      >
        <Suspense fallback={options.fallback ?? <div>Loading...</div>}>
          <LazyComponent {...props} />
        </Suspense>
      </ErrorBoundary>
    );
  };
}

// Usage
const Dashboard = loadComponent(
  () => import('./Dashboard'),
  {
    fallback: <DashboardSkeleton />,
    onError: (error) => console.error('Failed to load dashboard', error),
  }
);

Retry Logic for Failed Chunks

// utils/retryImport.ts
interface ImportRetryOptions {
  maxRetries?: number;
  delay?: number;
}

/**
 * Retry dynamic imports that fail (common with network issues)
 */
export async function retryImport<T>(
  factory: () => Promise<T>,
  { maxRetries = 3, delay = 1000 }: ImportRetryOptions = {}
): Promise<T> {
  let lastError: Error;

  for (let i = 0; i < maxRetries; i++) {
    try {
      return await factory();
    } catch (error) {
      lastError = error as Error;

      // Wait before retrying
      if (i < maxRetries - 1) {
        await new Promise(resolve => setTimeout(resolve, delay * (i + 1)));
      }
    }
  }

  throw lastError!;
}

// Usage
const Dashboard = lazy(() =>
  retryImport(() => import('./Dashboard'))
);

Prefetch Helper

// utils/prefetch.ts
/**
 * Prefetch a component without rendering it
 */
export function prefetchComponent(
  factory: () => Promise<{ default: ComponentType<any> }>
): void {
  factory().catch(() => {
    // Silently fail - prefetch is optimization, not critical
  });
}

// Prefetch on user interaction
function Navigation() {
  return (
    <nav>
      <a
        href="/dashboard"
        onMouseEnter={() => prefetchComponent(() => import('./pages/Dashboard'))}
      >
        Dashboard
      </a>
    </nav>
  );
}

Pattern 9: Bundle Analysis and Optimization

Analyzing Bundle Size

// Install bundle analyzer
// npm install --save-dev webpack-bundle-analyzer

// vite.config.ts
import { defineConfig } from 'vite';
import react from '@vitejs/plugin-react';
import { visualizer } from 'rollup-plugin-visualizer';

export default defineConfig({
  plugins: [
    react(),
    visualizer({
      open: true,
      gzipSize: true,
      brotliSize: true,
    }),
  ],
  build: {
    rollupOptions: {
      output: {
        manualChunks: {
          // Vendor splitting
          'react-vendor': ['react', 'react-dom'],
          'router': ['react-router-dom'],
          'query': ['@tanstack/react-query'],
        },
      },
    },
  },
});

Manual Chunk Configuration

// vite.config.ts
export default defineConfig({
  build: {
    rollupOptions: {
      output: {
        manualChunks(id) {
          // Split node_modules into separate chunks
          if (id.includes('node_modules')) {
            if (id.includes('react') || id.includes('react-dom')) {
              return 'react-vendor';
            }
            if (id.includes('lodash')) {
              return 'lodash-vendor';
            }
            if (id.includes('chart')) {
              return 'chart-vendor';
            }
            return 'vendor';
          }

          // Split pages into separate chunks
          if (id.includes('/pages/')) {
            const pageName = id.split('/pages/')[1].split('/')[0];
            return `page-${pageName}`;
          }
        },
      },
    },
  },
});

Pattern 10: Testing Lazy Components

Testing with Suspense

// Dashboard.test.tsx
import { render, screen, waitFor } from '@testing-library/react';
import { Suspense } from 'react';

// Import actual component for testing, not lazy version
import Dashboard from './Dashboard';

test('renders dashboard', async () => {
  render(
    <Suspense fallback={<div>Loading...</div>}>
      <Dashboard />
    </Suspense>
  );

  // Wait for suspense to resolve
  await waitFor(() => {
    expect(screen.getByText('Dashboard')).toBeInTheDocument();
  });
});

Mock Lazy Imports

// __mocks__/lazyComponent.ts
import { ComponentType } from 'react';

export function mockLazy<T extends ComponentType<any>>(
  component: T
): T {
  return component;
}

// Dashboard.test.tsx
jest.mock('./Dashboard', () => ({
  default: () => <div>Dashboard Mock</div>,
}));

test('renders mocked lazy component', () => {
  const Dashboard = lazy(() => import('./Dashboard'));

  render(
    <Suspense fallback={<div>Loading...</div>}>
      <Dashboard />
    </Suspense>
  );

  expect(screen.getByText('Dashboard Mock')).toBeInTheDocument();
});

Real-World Example: Complete Implementation

// App.tsx - Complete code splitting setup
import { lazy, Suspense, useEffect } from 'react';
import { Routes, Route, useLocation } from 'react-router-dom';
import { ErrorBoundary } from './components/ErrorBoundary';
import { retryImport } from './utils/retryImport';
import { preloadOnIdle } from './utils/preload';

// Lazy load pages
const HomePage = lazy(() => retryImport(() => import('./pages/Home')));
const DashboardPage = lazy(() => retryImport(() => import('./pages/Dashboard')));
const ProfilePage = lazy(() => retryImport(() => import('./pages/Profile')));
const SettingsPage = lazy(() => retryImport(() => import('./pages/Settings')));
const AdminPage = lazy(() => retryImport(() => import('./pages/Admin')));

// Preload map
const preloadMap: Record<string, () => void> = {
  '/': () => preloadOnIdle(() => import('./pages/Dashboard')),
  '/dashboard': () => preloadOnIdle(() => import('./pages/Profile')),
};

function App() {
  const location = useLocation();

  // Preload next likely page
  useEffect(() => {
    const preload = preloadMap[location.pathname];
    preload?.();
  }, [location.pathname]);

  return (
    <ErrorBoundary
      fallback={(error) => (
        <div>
          <h1>Application Error</h1>
          <p>{error.message}</p>
          <button onClick={() => window.location.reload()}>
            Reload
          </button>
        </div>
      )}
    >
      <Routes>
        <Route
          path="/"
          element={
            <Suspense fallback={<PageLoader />}>
              <HomePage />
            </Suspense>
          }
        />
        <Route
          path="/dashboard"
          element={
            <Suspense fallback={<PageLoader />}>
              <DashboardPage />
            </Suspense>
          }
        />
        <Route
          path="/profile"
          element={
            <Suspense fallback={<PageLoader />}>
              <ProfilePage />
            </Suspense>
          }
        />
        <Route
          path="/settings"
          element={
            <Suspense fallback={<PageLoader />}>
              <SettingsPage />
            </Suspense>
          }
        />
        <Route
          path="/admin"
          element={
            <Suspense fallback={<PageLoader />}>
              <AdminPage />
            </Suspense>
          }
        />
      </Routes>
    </ErrorBoundary>
  );
}

export default App;

Performance Metrics

Before Code Splitting

Initial Bundle: 2.5MB
Time to Interactive: 15s (3G)
Lighthouse Score: 23/100
First Contentful Paint: 8s

After Code Splitting

Initial Bundle: 180KB
Time to Interactive: 2s (3G)
Lighthouse Score: 94/100
First Contentful Paint: 1.2s

Route Chunks:
- Dashboard: 400KB
- Admin: 600KB
- Charts: 350KB

Best Practices

1. Split at Route Level First

Routes are natural split points. Users navigate between them.

2. Split Heavy Libraries

Chart.js, Markdown parsers, Rich text editors—split them all.

3. Don't Over-Split

Splitting every component creates overhead. Split things > 50KB.

4. Preload Aggressively

Preload on hover, idle time, or based on user patterns.

5. Monitor Bundle Sizes

Use bundle analyzer to find optimization opportunities.

6. Handle Errors Gracefully

Always wrap Suspense with ErrorBoundary.

7. Test Lazy Components

Test the actual components, not the lazy wrappers.

Common Mistakes

❌ Mistake 1: Splitting Too Small

// ❌ Bad - overhead exceeds benefit
const Button = lazy(() => import('./Button')); // 2KB component

❌ Mistake 2: No Error Handling

// ❌ Bad - crashes on failed chunk load
<Suspense fallback={<Loading />}>
  <Dashboard />
</Suspense>

❌ Mistake 3: Blocking Renders

// ❌ Bad - entire app waits for dashboard
function App() {
  const Dashboard = lazy(() => import('./Dashboard'));
  return <Dashboard />;
}

❌ Mistake 4: Not Using Type-Only Imports

// ❌ Bad - bundles types with code
import { User } from './types';

// ✅ Good - types are stripped
import type { User } from './types';

Conclusion

Code splitting transforms your app from a monolith to a nimble, fast-loading experience.

The strategy:

Split by routes first
Split heavy components second
Split libraries third
Preload aggressively
Monitor and optimize

The tools:

React.lazy() for components
Dynamic import() for utilities
Bundle analyzer for visibility
Preloading for perceived performance

The result:

Faster initial load
Better Time to Interactive
Happier users
Higher conversion rates

Start with routes. Measure with Lighthouse. Iterate based on data.

Your users on slow connections will thank you.

What's your bundle size? Run npm run build and share your before/after!

Compounded Components in TS. The way it components should be built :)

Tarun Moorjani — Sun, 18 Jan 2026 21:05:28 +0000

Compound Components in TypeScript: The Pattern That Scales

Tarun Moorjani ・ Jan 18

#react #typescript #webcomponents #programming

Compound Components in TypeScript: The Pattern That Scales

Tarun Moorjani — Sun, 18 Jan 2026 17:46:12 +0000

I used to build components like this:

<Select
  options={options}
  value={value}
  onChange={onChange}
  renderOption={(option) => <div>{option.label}</div>}
  renderTrigger={(selected) => <button>{selected?.label}</button>}
  placeholder="Select an option"
  disabled={false}
  error={error}
  // ... 20 more props
/>

Every new feature meant another prop. Every customization request meant more render functions. The component became a configuration nightmare with 30+ props, and it still couldn't handle all the use cases.

Then I discovered how Radix UI and Headless UI build their components. They use a pattern called compound components, and it changed how I think about component APIs.

<Select value={value} onChange={onChange}>
  <Select.Trigger>
    <Select.Value placeholder="Select an option" />
    <Select.Icon />
  </Select.Trigger>

  <Select.Content>
    {options.map(option => (
      <Select.Item key={option.id} value={option.id}>
        <Select.ItemText>{option.label}</Select.ItemText>
        <Select.ItemIndicator />
      </Select.Item>
    ))}
  </Select.Content>
</Select>

This isn't just prettier syntax. It's a fundamentally different approach to component design that scales infinitely while maintaining perfect type safety.

Let me show you how to build it.

What Are Compound Components?

Compound components are a set of components that work together to form a complete UI element. They share implicit state without prop drilling, and they compose naturally.

Think of HTML elements:

<select>
  <option value="1">Option 1</option>
  <option value="2">Option 2</option>
</select>

The <option> elements don't receive a selected prop. They implicitly know about the <select> parent's state. Compound components bring this pattern to React with full type safety.

Pattern 1: Basic Compound Components with Context

Let's build an Accordion component from scratch.

Step 1: Define the Types

// Accordion.types.ts
import { ReactNode } from 'react';

export interface AccordionContextValue {
  openItems: Set<string>;
  toggleItem: (id: string) => void;
  isOpen: (id: string) => boolean;
}

export interface AccordionProps {
  children: ReactNode;
  defaultOpen?: string[];
  multiple?: boolean;
  onChange?: (openItems: string[]) => void;
}

export interface AccordionItemProps {
  children: ReactNode;
  id: string;
}

export interface AccordionTriggerProps {
  children: ReactNode;
}

export interface AccordionContentProps {
  children: ReactNode;
}

Step 2: Create the Context

// Accordion.context.ts
import { createContext, useContext } from 'react';
import { AccordionContextValue } from './Accordion.types';

const AccordionContext = createContext<AccordionContextValue | null>(null);

export function useAccordionContext() {
  const context = useContext(AccordionContext);

  if (!context) {
    throw new Error(
      'Accordion compound components must be used within <Accordion>'
    );
  }

  return context;
}

export const AccordionProvider = AccordionContext.Provider;

Step 3: Build the Root Component

// Accordion.tsx
import { useState, useCallback } from 'react';
import { AccordionProvider } from './Accordion.context';
import { AccordionProps } from './Accordion.types';

export function Accordion({ 
  children, 
  defaultOpen = [],
  multiple = false,
  onChange 
}: AccordionProps) {
  const [openItems, setOpenItems] = useState<Set<string>>(
    new Set(defaultOpen)
  );

  const toggleItem = useCallback((id: string) => {
    setOpenItems(prev => {
      const next = new Set(prev);

      if (next.has(id)) {
        next.delete(id);
      } else {
        if (!multiple) {
          next.clear();
        }
        next.add(id);
      }

      onChange?.(Array.from(next));
      return next;
    });
  }, [multiple, onChange]);

  const isOpen = useCallback((id: string) => {
    return openItems.has(id);
  }, [openItems]);

  const contextValue = {
    openItems,
    toggleItem,
    isOpen,
  };

  return (
    <AccordionProvider value={contextValue}>
      <div className="accordion">
        {children}
      </div>
    </AccordionProvider>
  );
}

Step 4: Build Child Components

// AccordionItem.tsx
import { createContext, useContext, ReactNode } from 'react';

interface AccordionItemContextValue {
  id: string;
  isOpen: boolean;
}

const AccordionItemContext = createContext<AccordionItemContextValue | null>(null);

export function useAccordionItemContext() {
  const context = useContext(AccordionItemContext);

  if (!context) {
    throw new Error(
      'AccordionItem.* components must be used within <Accordion.Item>'
    );
  }

  return context;
}

interface AccordionItemProps {
  children: ReactNode;
  id: string;
}

export function AccordionItem({ children, id }: AccordionItemProps) {
  const { isOpen } = useAccordionContext();
  const itemIsOpen = isOpen(id);

  const contextValue = {
    id,
    isOpen: itemIsOpen,
  };

  return (
    <AccordionItemContext.Provider value={contextValue}>
      <div className="accordion-item" data-state={itemIsOpen ? 'open' : 'closed'}>
        {children}
      </div>
    </AccordionItemContext.Provider>
  );
}

// AccordionTrigger.tsx
export function AccordionTrigger({ children }: { children: ReactNode }) {
  const { toggleItem } = useAccordionContext();
  const { id, isOpen } = useAccordionItemContext();

  return (
    <button
      className="accordion-trigger"
      onClick={() => toggleItem(id)}
      aria-expanded={isOpen}
    >
      {children}
    </button>
  );
}

// AccordionContent.tsx
export function AccordionContent({ children }: { children: ReactNode }) {
  const { isOpen } = useAccordionItemContext();

  if (!isOpen) return null;

  return (
    <div className="accordion-content">
      {children}
    </div>
  );
}

Step 5: Export with Dot Notation

// index.ts
import { Accordion as AccordionRoot } from './Accordion';
import { AccordionItem } from './AccordionItem';
import { AccordionTrigger } from './AccordionTrigger';
import { AccordionContent } from './AccordionContent';

export const Accordion = Object.assign(AccordionRoot, {
  Item: AccordionItem,
  Trigger: AccordionTrigger,
  Content: AccordionContent,
});

// Usage - beautiful, composable API
function MyAccordion() {
  return (
    <Accordion defaultOpen={['item-1']}>
      <Accordion.Item id="item-1">
        <Accordion.Trigger>What is TypeScript?</Accordion.Trigger>
        <Accordion.Content>
          TypeScript is a typed superset of JavaScript.
        </Accordion.Content>
      </Accordion.Item>

      <Accordion.Item id="item-2">
        <Accordion.Trigger>Why use compound components?</Accordion.Trigger>
        <Accordion.Content>
          They provide flexible, composable APIs with implicit state sharing.
        </Accordion.Content>
      </Accordion.Item>
    </Accordion>
  );
}

Pattern 2: Type-Safe Dot Notation

Make TypeScript understand your compound component API.

// Proper typing for dot notation exports
type AccordionComponent = typeof AccordionRoot & {
  Item: typeof AccordionItem;
  Trigger: typeof AccordionTrigger;
  Content: typeof AccordionContent;
};

export const Accordion = Object.assign(AccordionRoot, {
  Item: AccordionItem,
  Trigger: AccordionTrigger,
  Content: AccordionContent,
}) as AccordionComponent;

// Now TypeScript knows:
// - Accordion is a component
// - Accordion.Item is a component
// - Accordion.Trigger is a component
// - etc.

Alternative: Namespace Approach

// Accordion.tsx
export function Accordion({ children }: AccordionProps) {
  // ... implementation
}

export namespace Accordion {
  export const Item = AccordionItem;
  export const Trigger = AccordionTrigger;
  export const Content = AccordionContent;
}

// Usage is identical
<Accordion>
  <Accordion.Item id="1">
    <Accordion.Trigger>Question</Accordion.Trigger>
    <Accordion.Content>Answer</Accordion.Content>
  </Accordion.Item>
</Accordion>

Pattern 3: Polymorphic Compound Components

Allow components to render as different elements.

// types/polymorphic.ts
import { ComponentPropsWithoutRef, ElementType, ReactNode } from 'react';

export type AsProp<C extends ElementType> = {
  as?: C;
};

export type PropsToOmit<C extends ElementType, P> = keyof (AsProp<C> & P);

export type PolymorphicComponentProp<
  C extends ElementType,
  Props = {}
> = Props &
  AsProp<C> &
  Omit<ComponentPropsWithoutRef<C>, PropsToOmit<C, Props>>;

export type PolymorphicRef<C extends ElementType> =
  ComponentPropsWithoutRef<C>['ref'];

Building a Polymorphic Trigger

// AccordionTrigger.tsx
import { forwardRef, ElementType } from 'react';
import {
  PolymorphicComponentProp,
  PolymorphicRef,
} from './types/polymorphic';

type AccordionTriggerProps<C extends ElementType = 'button'> =
  PolymorphicComponentProp<
    C,
    {
      children: ReactNode;
    }
  >;

export const AccordionTrigger = forwardRef(
  <C extends ElementType = 'button'>(
    { as, children, ...props }: AccordionTriggerProps<C>,
    ref?: PolymorphicRef<C>
  ) => {
    const Component = as || 'button';
    const { toggleItem } = useAccordionContext();
    const { id, isOpen } = useAccordionItemContext();

    return (
      <Component
        {...props}
        ref={ref}
        onClick={() => toggleItem(id)}
        aria-expanded={isOpen}
      >
        {children}
      </Component>
    );
  }
);

// Usage - fully typed!
<Accordion.Trigger>Default button</Accordion.Trigger>
<Accordion.Trigger as="div" role="button">Custom div</Accordion.Trigger>
<Accordion.Trigger as="a" href="#section">Link trigger</Accordion.Trigger>

// TypeScript knows which props are available:
<Accordion.Trigger 
  as="button" 
  type="button"  // ✅ Valid - button has type prop
  disabled       // ✅ Valid - button has disabled prop
/>

<Accordion.Trigger 
  as="div" 
  type="button"  // ❌ Error - div doesn't have type prop
  tabIndex={0}   // ✅ Valid - div has tabIndex
/>

Pattern 4: Generic Compound Components

Build components that work with any data type.

// Select component that works with any data
interface SelectProps<T> {
  value: T | null;
  onChange: (value: T) => void;
  children: ReactNode;
}

interface SelectContextValue<T> {
  value: T | null;
  onChange: (value: T) => void;
  isOpen: boolean;
  setIsOpen: (open: boolean) => void;
}

const SelectContext = createContext<SelectContextValue<any> | null>(null);

function useSelectContext<T>() {
  const context = useContext(SelectContext) as SelectContextValue<T> | null;

  if (!context) {
    throw new Error('Select components must be used within <Select>');
  }

  return context;
}

export function Select<T>({ value, onChange, children }: SelectProps<T>) {
  const [isOpen, setIsOpen] = useState(false);

  const contextValue: SelectContextValue<T> = {
    value,
    onChange,
    isOpen,
    setIsOpen,
  };

  return (
    <SelectContext.Provider value={contextValue}>
      <div className="select">{children}</div>
    </SelectContext.Provider>
  );
}

// Select.Item is generic too
interface SelectItemProps<T> {
  value: T;
  children: ReactNode;
}

function SelectItem<T>({ value, children }: SelectItemProps<T>) {
  const { value: selectedValue, onChange, setIsOpen } = useSelectContext<T>();
  const isSelected = selectedValue === value;

  return (
    <button
      className="select-item"
      data-selected={isSelected}
      onClick={() => {
        onChange(value);
        setIsOpen(false);
      }}
    >
      {children}
    </button>
  );
}

Select.Item = SelectItem;

// Usage - full type inference!
interface User {
  id: string;
  name: string;
}

const users: User[] = [
  { id: '1', name: 'Alice' },
  { id: '2', name: 'Bob' },
];

<Select value={selectedUser} onChange={setSelectedUser}>
  <Select.Trigger>
    {selectedUser?.name ?? 'Select user'}
  </Select.Trigger>
  <Select.Content>
    {users.map(user => (
      <Select.Item key={user.id} value={user}>
        {/* TypeScript knows user is User */}
        {user.name}
      </Select.Item>
    ))}
  </Select.Content>
</Select>

Pattern 5: Enforcing Component Structure

Prevent invalid component nesting.

// Tabs component that validates structure
interface TabsContextValue {
  activeTab: string;
  setActiveTab: (id: string) => void;
  tabs: Set<string>;
  registerTab: (id: string) => void;
  unregisterTab: (id: string) => void;
}

export function Tabs({ children, defaultTab }: TabsProps) {
  const [activeTab, setActiveTab] = useState(defaultTab);
  const [tabs, setTabs] = useState<Set<string>>(new Set());

  const registerTab = useCallback((id: string) => {
    setTabs(prev => new Set(prev).add(id));
  }, []);

  const unregisterTab = useCallback((id: string) => {
    setTabs(prev => {
      const next = new Set(prev);
      next.delete(id);
      return next;
    });
  }, []);

  const contextValue = {
    activeTab,
    setActiveTab,
    tabs,
    registerTab,
    unregisterTab,
  };

  return (
    <TabsContext.Provider value={contextValue}>
      {children}
    </TabsContext.Provider>
  );
}

// TabPanel auto-registers
export function TabPanel({ id, children }: TabPanelProps) {
  const { registerTab, unregisterTab, activeTab } = useTabsContext();

  useEffect(() => {
    registerTab(id);
    return () => unregisterTab(id);
  }, [id, registerTab, unregisterTab]);

  if (activeTab !== id) return null;

  return <div className="tab-panel">{children}</div>;
}

// TabTrigger validates tab exists
export function TabTrigger({ id, children }: TabTriggerProps) {
  const { setActiveTab, activeTab, tabs } = useTabsContext();

  // Warning if tab doesn't exist
  useEffect(() => {
    if (!tabs.has(id)) {
      console.warn(`Tab "${id}" doesn't have a corresponding TabPanel`);
    }
  }, [id, tabs]);

  return (
    <button
      className="tab-trigger"
      data-active={activeTab === id}
      onClick={() => setActiveTab(id)}
    >
      {children}
    </button>
  );
}

Pattern 6: Render Props with Compound Components

Combine patterns for maximum flexibility.

// Accordion with render props
interface AccordionItemRenderProps {
  isOpen: boolean;
  toggle: () => void;
}

interface AccordionItemProps {
  id: string;
  children: (props: AccordionItemRenderProps) => ReactNode;
}

export function AccordionItem({ id, children }: AccordionItemProps) {
  const { isOpen, toggleItem } = useAccordionContext();
  const itemIsOpen = isOpen(id);

  return (
    <div className="accordion-item">
      {children({
        isOpen: itemIsOpen,
        toggle: () => toggleItem(id),
      })}
    </div>
  );
}

// Usage - full control over rendering
<Accordion>
  <Accordion.Item id="item-1">
    {({ isOpen, toggle }) => (
      <>
        <button onClick={toggle}>
          Question {isOpen ? '▼' : '▶'}
        </button>
        {isOpen && <div>Answer</div>}
      </>
    )}
  </Accordion.Item>
</Accordion>

Pattern 7: Controlled vs Uncontrolled

Support both patterns with TypeScript.

// Union type for controlled vs uncontrolled
type ControlledProps = {
  value: string;
  onChange: (value: string) => void;
  defaultValue?: never;
};

type UncontrolledProps = {
  value?: never;
  onChange?: never;
  defaultValue: string;
};

type SelectProps = (ControlledProps | UncontrolledProps) & {
  children: ReactNode;
};

export function Select(props: SelectProps) {
  const isControlled = props.value !== undefined;

  const [internalValue, setInternalValue] = useState(
    props.defaultValue || ''
  );

  const value = isControlled ? props.value : internalValue;

  const onChange = isControlled
    ? props.onChange
    : setInternalValue;

  // Rest of implementation uses value and onChange
  // ...
}

// TypeScript enforces correct usage:

// ✅ Controlled
<Select value={value} onChange={setValue}>...</Select>

// ✅ Uncontrolled
<Select defaultValue="option1">...</Select>

// ❌ Mixed (TypeScript error)
<Select value={value} defaultValue="option1">...</Select>

// ❌ Missing onChange when controlled
<Select value={value}>...</Select>

Pattern 8: Forwarding Refs Through Compound Components

// AccordionTrigger with ref forwarding
export const AccordionTrigger = forwardRef<
  HTMLButtonElement,
  AccordionTriggerProps
>(({ children, ...props }, ref) => {
  const { toggleItem } = useAccordionContext();
  const { id, isOpen } = useAccordionItemContext();

  return (
    <button
      {...props}
      ref={ref}
      onClick={() => toggleItem(id)}
      aria-expanded={isOpen}
    >
      {children}
    </button>
  );
});

AccordionTrigger.displayName = 'AccordionTrigger';

// Usage with ref
function MyComponent() {
  const triggerRef = useRef<HTMLButtonElement>(null);

  useEffect(() => {
    triggerRef.current?.focus();
  }, []);

  return (
    <Accordion>
      <Accordion.Item id="1">
        <Accordion.Trigger ref={triggerRef}>
          Question
        </Accordion.Trigger>
        <Accordion.Content>Answer</Accordion.Content>
      </Accordion.Item>
    </Accordion>
  );
}

Pattern 9: Slot-Based Composition

Alternative pattern using named slots.

// Card with slots
interface CardProps {
  children: ReactNode;
}

interface CardSlotContextValue {
  slots: {
    header?: ReactNode;
    content?: ReactNode;
    footer?: ReactNode;
  };
  registerSlot: (name: string, content: ReactNode) => void;
}

const CardSlotContext = createContext<CardSlotContextValue | null>(null);

export function Card({ children }: CardProps) {
  const [slots, setSlots] = useState<CardSlotContextValue['slots']>({});

  const registerSlot = useCallback((name: string, content: ReactNode) => {
    setSlots(prev => ({ ...prev, [name]: content }));
  }, []);

  return (
    <CardSlotContext.Provider value={{ slots, registerSlot }}>
      {/* Process children to register slots */}
      <div style={{ display: 'none' }}>{children}</div>

      {/* Render slots in specific order */}
      <div className="card">
        {slots.header && <div className="card-header">{slots.header}</div>}
        {slots.content && <div className="card-content">{slots.content}</div>}
        {slots.footer && <div className="card-footer">{slots.footer}</div>}
      </div>
    </CardSlotContext.Provider>
  );
}

function CardHeader({ children }: { children: ReactNode }) {
  const { registerSlot } = useContext(CardSlotContext)!;

  useEffect(() => {
    registerSlot('header', children);
  }, [children, registerSlot]);

  return null;
}

function CardContent({ children }: { children: ReactNode }) {
  const { registerSlot } = useContext(CardSlotContext)!;

  useEffect(() => {
    registerSlot('content', children);
  }, [children, registerSlot]);

  return null;
}

Card.Header = CardHeader;
Card.Content = CardContent;

// Usage - order doesn't matter!
<Card>
  <Card.Content>This renders second</Card.Content>
  <Card.Header>This renders first</Card.Header>
</Card>

Pattern 10: Complex State Management

Use useReducer for complex compound component state.

// Dropdown with complex state
type DropdownState = {
  isOpen: boolean;
  selectedIndex: number;
  searchQuery: string;
  highlightedIndex: number;
};

type DropdownAction =
  | { type: 'OPEN' }
  | { type: 'CLOSE' }
  | { type: 'TOGGLE' }
  | { type: 'SELECT'; index: number }
  | { type: 'HIGHLIGHT'; index: number }
  | { type: 'SEARCH'; query: string }
  | { type: 'KEYBOARD_NAVIGATE'; direction: 'up' | 'down' }
  | { type: 'RESET' };

function dropdownReducer(
  state: DropdownState,
  action: DropdownAction
): DropdownState {
  switch (action.type) {
    case 'OPEN':
      return { ...state, isOpen: true };

    case 'CLOSE':
      return { ...state, isOpen: false, searchQuery: '' };

    case 'TOGGLE':
      return { ...state, isOpen: !state.isOpen };

    case 'SELECT':
      return {
        ...state,
        selectedIndex: action.index,
        isOpen: false,
        searchQuery: '',
      };

    case 'HIGHLIGHT':
      return { ...state, highlightedIndex: action.index };

    case 'SEARCH':
      return { ...state, searchQuery: action.query };

    case 'KEYBOARD_NAVIGATE':
      const direction = action.direction === 'up' ? -1 : 1;
      return {
        ...state,
        highlightedIndex: Math.max(
          0,
          Math.min(state.highlightedIndex + direction, /* itemCount - 1 */ 10)
        ),
      };

    case 'RESET':
      return {
        isOpen: false,
        selectedIndex: -1,
        searchQuery: '',
        highlightedIndex: -1,
      };

    default:
      return state;
  }
}

export function Dropdown({ children }: { children: ReactNode }) {
  const [state, dispatch] = useReducer(dropdownReducer, {
    isOpen: false,
    selectedIndex: -1,
    searchQuery: '',
    highlightedIndex: -1,
  });

  const contextValue = {
    state,
    dispatch,
  };

  return (
    <DropdownContext.Provider value={contextValue}>
      {children}
    </DropdownContext.Provider>
  );
}

Real-World Example: Complete Select Component

// Select/index.tsx - Full implementation
import { useState, useRef, useEffect, ReactNode, forwardRef } from 'react';

// Types
interface SelectContextValue<T> {
  value: T | null;
  onChange: (value: T) => void;
  isOpen: boolean;
  setIsOpen: (open: boolean) => void;
  triggerRef: React.RefObject<HTMLButtonElement>;
}

// Context
const SelectContext = createContext<SelectContextValue<any> | null>(null);

function useSelectContext<T>() {
  const context = useContext(SelectContext);
  if (!context) {
    throw new Error('Select.* components must be used within <Select>');
  }
  return context as SelectContextValue<T>;
}

// Root Component
interface SelectProps<T> {
  value: T | null;
  onChange: (value: T) => void;
  children: ReactNode;
}

function SelectRoot<T>({ value, onChange, children }: SelectProps<T>) {
  const [isOpen, setIsOpen] = useState(false);
  const triggerRef = useRef<HTMLButtonElement>(null);

  // Close on outside click
  useEffect(() => {
    if (!isOpen) return;

    const handleClick = (e: MouseEvent) => {
      if (!triggerRef.current?.contains(e.target as Node)) {
        setIsOpen(false);
      }
    };

    document.addEventListener('click', handleClick);
    return () => document.removeEventListener('click', handleClick);
  }, [isOpen]);

  const contextValue: SelectContextValue<T> = {
    value,
    onChange,
    isOpen,
    setIsOpen,
    triggerRef,
  };

  return (
    <SelectContext.Provider value={contextValue}>
      <div className="select">{children}</div>
    </SelectContext.Provider>
  );
}

// Trigger Component
interface SelectTriggerProps {
  children: ReactNode;
}

const SelectTrigger = forwardRef<HTMLButtonElement, SelectTriggerProps>(
  ({ children }, ref) => {
    const { isOpen, setIsOpen, triggerRef } = useSelectContext();

    return (
      <button
        ref={(node) => {
          (triggerRef as React.MutableRefObject<HTMLButtonElement | null>).current = node;
          if (typeof ref === 'function') ref(node);
          else if (ref) ref.current = node;
        }}
        className="select-trigger"
        onClick={() => setIsOpen(!isOpen)}
        aria-expanded={isOpen}
      >
        {children}
      </button>
    );
  }
);

// Value Component
function SelectValue({ placeholder }: { placeholder?: string }) {
  const { value } = useSelectContext();

  if (value === null) {
    return <span className="select-placeholder">{placeholder}</span>;
  }

  // User provides the display via children in SelectItem
  return null;
}

// Content Component
function SelectContent({ children }: { children: ReactNode }) {
  const { isOpen } = useSelectContext();

  if (!isOpen) return null;

  return (
    <div className="select-content">
      {children}
    </div>
  );
}

// Item Component
interface SelectItemProps<T> {
  value: T;
  children: ReactNode;
}

function SelectItem<T>({ value, children }: SelectItemProps<T>) {
  const { value: selectedValue, onChange, setIsOpen } = useSelectContext<T>();
  const isSelected = selectedValue === value;

  return (
    <button
      className="select-item"
      data-selected={isSelected}
      onClick={() => {
        onChange(value);
        setIsOpen(false);
      }}
    >
      {children}
      {isSelected && <span className="select-checkmark">✓</span>}
    </button>
  );
}

// Export with dot notation
export const Select = Object.assign(SelectRoot, {
  Trigger: SelectTrigger,
  Value: SelectValue,
  Content: SelectContent,
  Item: SelectItem,
});

// Usage
interface Option {
  id: string;
  label: string;
}

function App() {
  const [selected, setSelected] = useState<Option | null>(null);

  const options: Option[] = [
    { id: '1', label: 'Option 1' },
    { id: '2', label: 'Option 2' },
    { id: '3', label: 'Option 3' },
  ];

  return (
    <Select value={selected} onChange={setSelected}>
      <Select.Trigger>
        <Select.Value placeholder="Select an option" />
        {selected && <span>{selected.label}</span>}
      </Select.Trigger>

      <Select.Content>
        {options.map(option => (
          <Select.Item key={option.id} value={option}>
            {option.label}
          </Select.Item>
        ))}
      </Select.Content>
    </Select>
  );
}

Advantages Over Alternative Patterns

vs. Monolithic Components

// ❌ Monolithic - inflexible
<Select
  options={options}
  renderOption={(opt) => <div>{opt.label}</div>}
  renderTrigger={(val) => <button>{val?.label}</button>}
  icon={<ChevronIcon />}
  searchable={true}
  clearable={true}
  // 20+ more props
/>

// ✅ Compound - composable
<Select value={value} onChange={onChange}>
  <Select.Trigger>
    {value?.label ?? 'Select'}
    <ChevronIcon />
  </Select.Trigger>
  <Select.Content>
    <Select.Search />
    {options.map(opt => (
      <Select.Item value={opt}>{opt.label}</Select.Item>
    ))}
  </Select.Content>
</Select>

vs. Render Props

// ❌ Render props - nesting hell
<Select value={value} onChange={onChange}>
  {({ isOpen, selectedValue }) => (
    <>
      <SelectTrigger>
        {({ toggle }) => (
          <button onClick={toggle}>
            {selectedValue?.label ?? 'Select'}
          </button>
        )}
      </SelectTrigger>
      {isOpen && (
        <SelectContent>
          {({ selectOption }) => (
            options.map(opt => (
              <button onClick={() => selectOption(opt)}>
                {opt.label}
              </button>
            ))
          )}
        </SelectContent>
      )}
    </>
  )}
</Select>

// ✅ Compound - flat and readable
<Select value={value} onChange={onChange}>
  <Select.Trigger>
    <Select.Value />
  </Select.Trigger>
  <Select.Content>
    {options.map(opt => (
      <Select.Item value={opt}>{opt.label}</Select.Item>
    ))}
  </Select.Content>
</Select>

Best Practices

1. Clear Component Names

// ✅ Good - obvious hierarchy
<Accordion>
  <Accordion.Item>
    <Accordion.Trigger />
    <Accordion.Content />
  </Accordion.Item>
</Accordion>

// ❌ Bad - unclear relationship
<Accordion>
  <Item>
    <Button />
    <Panel />
  </Item>
</Accordion>

2. Throw Helpful Errors

function useAccordionContext() {
  const context = useContext(AccordionContext);

  if (!context) {
    throw new Error(
      'Accordion.* components must be used within <Accordion>. ' +
      'Did you forget to wrap your components?'
    );
  }

  return context;
}

3. Document Component Structure

/**
 * Accordion component for collapsible content sections.
 * 
 * @example
 * ```

tsx
 * <Accordion defaultOpen={['item-1']}>
 *   <Accordion.Item id="item-1">
 *     <Accordion.Trigger>Question</Accordion.Trigger>
 *     <Accordion.Content>Answer</Accordion.Content>
 *   </Accordion.Item>
 * </Accordion>
 *

/
export const Accordion = { / ... */ };




### 4. Support Composition



```typescript
// Allow custom components between layers
<Accordion>
  <div className="accordion-section">
    <h2>Section Title</h2>
    <Accordion.Item id="1">
      <Accordion.Trigger>Question</Accordion.Trigger>
      <Accordion.Content>Answer</Accordion.Content>
    </Accordion.Item>
  </div>
</Accordion>

When to Use Compound Components

Use compound components when:

Components have complex internal state
Multiple child components need to share state
You want maximum customization flexibility
The component hierarchy is important
You're building a design system

Don't use compound components when:

A simple component with 3-5 props would work
There's no shared state between children
The API would be confusing for your use case
You're building a one-off component

Conclusion

Compound components transform component APIs from configuration nightmares into composable, flexible systems.

Instead of:

30+ props
Complex render prop functions
Limited customization
Prop drilling

You get:

Implicit state sharing
Infinite customization
Clear component hierarchy
Perfect TypeScript support

The pattern takes more code upfront, but it scales infinitely. Every design system library uses it for a reason.

Start with a simple compound component. Build an Accordion or Tabs. Once you see how the pattern works, you'll wonder how you ever lived without it.

What components would benefit from the compound pattern in your codebase? Share your ideas!