You're halfway through a React PR review and everything looks plausible. The logic reads fine, the component names make sense, and the tests pass. Then the feature ships and the list re-renders with the wrong item selected, or the fetch silently fails on a slow connection, or a keyboard-only user can't activate the button at all.
These aren't obscure edge cases. They're patterns that appear in React codebases at every level, and they're easy to miss precisely because the code around them looks correct. A reviewer who knows what to look for catches them before they reach production.
The ten items below are the ones worth building into your review checklist.
1. Missing keys in lists, or using index as key
React uses the key prop to track which list items changed between renders. When you use the array index as a key, React ties each item's identity to its position, not its data. Delete the second item in a list of three and React sees the third item slide into position two, inheriting whatever state or DOM the previous item left behind.
Imagine a to-do list where each item has a controlled input for an inline edit. If item 2 is deleted, item 3 moves to index 1 and picks up item 2's input state. The data is gone but the UI shows the wrong value. This is a silent data integrity bug.
The fix is a stable, unique ID from the data itself: a database ID, a UUID, anything that doesn't shift when the list changes. The one acceptable exception is a static list that never reorders and has no stateful children, where index keys are harmless.
2. useEffect with a missing or wrong dependency array
There are two ways this breaks. An empty dependency array with a closure over props or state means the effect captures the values from the first render and never updates, producing stale data that's hard to trace. A dependency array that's missing a value that changes on every render causes the effect to fire in an infinite loop.
The pattern reviewers should always surface is a suppressed exhaustive-deps ESLint rule. A comment like // eslint-disable-next-line react-hooks/exhaustive-deps is a flag, not a solution. It usually means the developer couldn't make the deps array work and decided to silence the warning instead.
The real fix often requires restructuring: extracting the relevant logic, memoising a dependency, or splitting the effect. Just adding the missing dep to the array sometimes works, but sometimes it only moves the problem. Either way, the suppression comment shouldn't be left in the code without a clear explanation of why it's justified.
3. State updates that mutate the existing object
React's re-render logic depends on referential equality. If you mutate an object or array in state and set it back, React compares the old reference to the new one, sees the same object, and skips the render. The data changed; the screen didn't.
The classic version is pushing to an array: state.items.push(newItem); setState(state.items). It looks like a state update. It isn't. The spread pattern, setState([...state.items, newItem]), creates a new reference React can detect.
Nested objects are harder. A mutation two levels deep is easy to miss at a glance, especially when the rest of the function is straightforward. This bug hides in code review because the intent is clear and the logic appears sound. Look for any assignment to a property of a state variable, or any array method that modifies in place: push, pop, splice, sort, reverse.
4. Props passed down through too many layers without context or composition
Prop drilling becomes a reviewer concern when a prop passes through three or more components that don't use it, only forwarding it to a child that does. The intermediate components become coupled to data they don't own, and refactoring any layer in the chain requires touching every component in the path.
The practical fixes depend on scope. React Context works well for values that are genuinely shared across a subtree. Component composition, passing children or render props instead of data, often solves the problem without any state management at all. A dedicated state library makes sense for data that's truly global and changes frequently.
Prop drilling isn't always wrong. In a shallow two-level tree, adding context overhead isn't worth it. The reviewer's question is whether the intermediate components should actually know about this prop, or whether the architecture is just passing it through because nothing better was considered.
5. Expensive calculations run on every render without useMemo
Not every calculation that looks expensive actually is. Reviewers should be suspicious of useMemo added everywhere, because it carries its own overhead: the memoised value has to be stored, the dependency array has to be compared on each render, and the code becomes harder to read. Wrapping a simple string concatenation in useMemo is worse than not wrapping it.
The real targets are sorting or filtering large arrays, deriving complex objects from multiple inputs, or any calculation that feeds directly into a memoised child component's props. The honest check is the React profiler, not intuition. If you can't point to a profiler trace showing the render time, the useMemo is probably noise.
When you see a PR adding useMemo to every derived value, that's worth flagging as cargo-cult optimisation, not performance work.
6. Event handlers recreated on every render without useCallback
useCallback matters in one specific situation: when a handler is passed as a prop to a child wrapped in React.memo. Without useCallback, a new function reference is created on every render, which defeats the memoisation and causes the child to re-render anyway.
Outside that scenario, useCallback is usually noise. A handler that's only used inside the component doesn't benefit from being wrapped. Reviewers should apply a quick test: if the child component isn't wrapped in React.memo, the useCallback almost certainly isn't doing anything useful.
The pattern to flag in review is a component where every single function is wrapped in useCallback regardless of how it's used. It suggests a misunderstanding of what the hook actually does, and it adds visual and cognitive weight to code that didn't need it.
7. Fetching data directly in a component without handling loading and error states
The most common version looks like this: a useEffect with a fetch call, the result set into state, no loading boolean, and no catch block. On a fast connection it seems fine. On a slow one, the component renders empty while the request is in flight. If the request fails, nothing happens: no error message, no retry, just silence.
There's also the unmount problem. If the component unmounts before the fetch resolves and the callback still tries to call setState, React logs a warning and the behaviour becomes unpredictable. The fix is a cleanup function in the effect that cancels the request or ignores the result after unmount.
Libraries like React Query and SWR handle loading state, error state, caching, and cleanup as first-class concerns. Recommending them in review is reasonable when you're looking at a component doing all of this by hand. Which one is worth suggesting depends on what's already in the project.
8. Components with no error boundary around unpredictable subtrees
A thrown error anywhere in a React component tree, without an error boundary above it, unmounts the entire app. The user sees a blank screen. There's no fallback, no message, no recovery path.
The subtrees that need boundaries most are ones rendering data you don't fully control: third-party widgets, user-generated content, async data that might arrive in an unexpected shape. Reviewers should also look at any PR adding a new top-level route or page: if it doesn't have an error boundary, a runtime error in that route takes the whole app down, not just that page.
Error boundaries still require class components or a library like react-error-boundary. That trips up developers who've worked mostly with function components. Pointing this out in review, with a note on what the boundary should show as a fallback, is a concrete and useful comment rather than a vague style note. Giving feedback that's specific enough to act on makes the difference between a comment that helps and one that gets dismissed.
9. Accessibility attributes skipped on interactive elements
The most common offender is a div or span wired with an onClick handler. It works with a mouse. With a keyboard, it's unreachable: no tab stop, no Enter activation, no focus indicator. A screen reader announces it as a generic container with no role.
Using a button element costs nothing and gives you all of that for free. The same goes for a tags: if it navigates somewhere, it should be an anchor with an href, not a div with a click handler and a pointer cursor.
A separate but equally common miss is icon-only buttons with no aria-label. A button that renders a trash icon and nothing else is announced to screen readers as "button" with no description of what it does. One attribute fixes it.
The reviewer's practical rule: if it's interactive and it isn't a native interactive element, it needs explicit ARIA. Security and access issues in PRs often appear in the same places, so reviewing these elements carefully pays off twice.
10. Hardcoded styles and magic numbers buried in components
A value like marginTop: 23 or opacity: 0.87 in a component tells the next developer nothing. Why 23? Why not 24? Without context, nobody knows whether it's intentional, a coincidence, or a leftover from manual tweaking. Over time, the same value appears in a dozen components with slight variations, dark mode breaks unpredictably, and design tokens become decorative.
Magic numbers in styles are the same problem as magic numbers in logic: they carry no intent. The reviewer's question is a single one: does this value belong in a design token, a named constant, or at minimum a comment explaining why it's that specific number?
This is a maintainability flag, not a bug. Frame it that way in review comments. It's not wrong today, it's a maintenance cost that compounds. A comment like "should this live in our spacing tokens?" is more useful than a rejection, and it teaches the pattern without being heavy-handed. Pairing this kind of observation with catching logic bugs early makes your reviews consistently valuable rather than inconsistently strict.
These ten patterns show up across React codebases at every experience level. Knowing them by name is useful. Recognising them under pressure, in unfamiliar code, with a real PR waiting, is a different skill. It's one you build by practice.
Try a graded React code review on Goodcatch and see what you catch.