My React code review checklist: best practices explained

After reviewing hundreds of pull requests over the last five years, I've learned that most bugs aren't syntax errors—they're architectural decisions made in isolation. Today, I'm sharing my React code review checklist, a practical framework I use to catch issues before they ship to production. Whether you're reviewing a teammate's code or your own, these checks will help you spot problems early and maintain a sustainable codebase.

State — Minimal, derived where possible, in the right place

The first thing I check is state shape. Too many React developers treat component state like a dump for every value they might need later. I look for three patterns: states that should be derived, states that are stored in the wrong place, and states that could disappear entirely.

If I see this:

const [user, setUser] = useState(null);
const [firstName, setFirstName] = useState('');
const [lastName, setLastName] = useState('');
const [fullName, setFullName] = useState('');

useEffect(() => {
  if (user) {
    setFirstName(user.first_name);
    setLastName(user.last_name);
    setFullName(`${user.first_name} ${user.last_name}`);
  }
}, [user]);

I stop and ask: Why are firstName, lastName, and fullName stored separately? They're all derived from user. This adds synchronization burden and creates three sources of truth. The fix is straightforward:

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

const firstName = user?.first_name ?? '';
const lastName = user?.last_name ?? '';
const fullName = `${firstName} ${lastName}`;

I also check placement. Does this state belong in this component, or should it live higher in the tree? A common mistake is storing filter state in the child that displays results when the parent needs it too. I ask: what happens if the user navigates away and back? If the filter should persist, it might belong in a URL param or context, not local state.

The rule I follow: store the minimum, derive everything else, and place state where its consumers live.

Effects — Justified, cleaned up, no data syncing

Effects are where I spend the most review time. They're powerful and easy to misuse. I look for three red flags: effects without justification, missing cleanup, and effects that try to keep state synchronized.

First, I check if an effect is necessary at all. Does this code fetch data? Listen to a WebSocket? Subscribe to external events? Those justify an effect. But I've seen effects that run on every render just to transform data:

useEffect(() => {
  const sorted = items.sort((a, b) => a.name.localeCompare(b.name));
  setSorted(sorted);
}, [items]);

That's unnecessary. Sorting happens on every render, then the effect runs, then another render happens. Instead, sort inline or wrap it in useMemo if it's truly expensive.

Second, I verify cleanup. If an effect sets up a listener or timer, does it clean up? This one fails code review every time:

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

The fix is trivial but critical:

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

Third, I look for data synchronization attempts. This pattern is almost always wrong:

const [email, setEmail] = useState('');

useEffect(() => {
  setEmail(user?.email ?? '');
}, [user]);

You're keeping email in sync with user manually. That's what deriving does. Just use const email = user?.email ?? ''; instead. The only time data synchronization via effects makes sense is when you're syncing React state to an external system, like localStorage or a ref.

Performance — No heavy work in render, lists keyed, images sized

I check three performance patterns: expensive computation during render, lists without proper keys, and images without fixed dimensions.

First, expensive work. If something takes time, it shouldn't block render. I look for filtering, sorting, or transforming large datasets inline:

const expensiveList = items
  .filter(item => item.category === selectedCategory)
  .sort((a, b) => a.price - b.price)
  .map(item => ({ ...item, discount: item.price * 0.9 }));

If items is 10,000 elements and this rerenders frequently, you've got a problem. Wrap it in useMemo:

const expensiveList = useMemo(
  () => items
    .filter(item => item.category === selectedCategory)
    .sort((a, b) => a.price - b.price)
    .map(item => ({ ...item, discount: item.price * 0.9 })),
  [items, selectedCategory]
);

Second, list keys. I always check that lists render with unique, stable keys. This is wrong:

items.map((item, index) => (
  <li>{item.name}</li>
))

If the list reorders or items insert/delete, using index as a key breaks component state and causes subtle bugs. Use a unique identifier from your data:

items.map((item) => (
  <li>{item.name}</li>
))

Third, images. Unoptimized images cause layout shift and poor Core Web Vitals. With Next.js, I always expect the Image component with width and height. Without Next.js, images should have explicit dimensions:

// Bad
<img src="/photo.jpg" alt="photo" />

// Good
<img src="/photo.jpg" alt="photo" width="{800}" height="{600}" />

Accessibility — Semantic elements, labels, keyboard support

Accessibility isn't optional. I check three things: semantic HTML, associated labels, and keyboard navigation.

Semantic HTML first. I see too many divs pretending to be buttons:

// Bad
<div>
  Save
</div>

// Good

  Save

A real button gets keyboard support, focus styles, and screen reader announcements for free. Use button, link, nav, section, article, form instead of div soup.

Labels come next. Every form input needs a label:

// Bad


// Good
Email

Placeholder text isn't a label. It disappears when you start typing. Screen readers ignore it. A proper label is clickable and associated via htmlFor.

Keyboard support means testing tab order and ensuring interactive elements are reachable. If you're building a custom dropdown, modal, or menu, ensure you can navigate it with arrow keys and dismiss it with Escape.

Boundaries — Client components small, secrets on the server

This applies to Next.js specifically. I check that "use client" boundaries are tight and that secrets stay server-side.

A common mistake is marking an entire page as a client component when only a small interactive part needs it:

// Bad: marks entire page as client
'use client';

export default function ProductPage({ product }) {
  return (
    <div>
      <h1>{product.title}</h1>
      <p>{product.description}</p>
      
    </div>
  );
}

Instead, extract the interactive part:

// Server component
export default function ProductPage({ product }) {
  return (
    <div>
      <h1>{product.title}</h1>
      <p>{product.description}</p>
      
    </div>
  );
}

// Client component (small, focused)
'use client';
function LikeButton({ productId }) {
  return  likeProduct(productId)}>Like;
}

Second, I check that API keys, database credentials, and private data never end up in client code. This is easy to miss if you're not thinking about it. A private API key should never be imported in a client component, even inside a function. Use server actions or an API route instead.

Common mistakes

• Forgetting dependency arrays: useEffect(() => {}, []) means once. useEffect(() => {}) means every render. Missing the array entirely causes infinite loops or stale data.
• Spreading props carelessly: {...props} can override intentional values and introduce unexpected behavior. Be explicit about which props you pass.
• Mutating state directly: user.name = 'John' won't trigger a rerender. Always create a new object: setUser({...user, name: 'John'}).
• Not testing edge cases: What happens if the API is slow? If data is null? If the array is empty? I check for defensive rendering.
• Ignoring console warnings: Missing keys, missing dependencies, and other React warnings exist for a reason. Fix them during code review, not in production.

Key takeaways

• State should be minimal and derive everything possible. Place it where it's used or needed for persistence.
• Effects need justification. If it's not fetching, listening, or syncing to an external system, it probably shouldn't exist.
• Performance matters for every user. Memoize expensive work, key lists correctly, and size images.
• Accessibility is a responsibility. Use semantic HTML, associate labels, and support keyboard navigation.
• In Next.js, keep client components small and never expose secrets to the browser.

This checklist has caught hundreds of bugs before they shipped. Make it your own, adjust it to your team's standards, and use it consistently. Good code review is about preventing problems, not just catching them.