Code review example: memo defeated: a fresh callback per render re-renders every row anyway in React

Retired from the DiffDojo practice pool in October 2026; the review below is the canonical answer. An AI wrote this pull request for the task underneath. Read the diff and decide what you would say before you scroll to the review.

React · Performance · intermediate

The task

Filterable metrics table

  1. Add a metrics table with a text filter over metric names.
  2. Clicking a row selects that metric.
  3. Typing in the filter must re-render only rows whose data changed, row rendering is expensive.

Adds the metrics table with client-side filtering and row selection. Rows are memoized with React.memo so the expensive sparkline cells don't re-render while typing.

The dashboard shows up to 5,000 metrics; each row's Sparkline renders an SVG path from ~200 points.

The diff

src/components/MetricsTable.jsx+40
1+ import { memo, useState } from "react";
2+ import { Sparkline } from "./Sparkline";
3+
4+ const Row = memo(function Row({ metric, onSelect }) {
5+ return (
6+ <tr onClick={() => onSelect(metric.id)}>
7+ <td>{metric.name}</td>
8+ <td><Sparkline points={metric.history} /></td>
9+ </tr>
10+ );
11+ });
12+
13+ export function MetricsTable({ metrics }) {
14+ const [query, setQuery] = useState("");
15+ const [selectedId, setSelectedId] = useState(null);
16+
17+ const visible = metrics.filter((m) => m.name.includes(query));
18+
19+ return (
20+ <div>
21+ <input
22+ value={query}
23+ onChange={(e) => setQuery(e.target.value)}
24+ placeholder="Filter metrics…"
25+ />
26+ <table>
27+ <tbody>
28+ {visible.map((metric) => (
29+ <Row
30+ key={metric.id}
31+ metric={metric}
32+ onSelect={(id) => setSelectedId(id)}
33+ />
34+ ))}
35+ </tbody>
36+ </table>
37+ <p>{selectedId ? `Selected: ${selectedId}` : "Click a row"}</p>
38+ </div>
39+ );
40+ }

The review

Issue 1: memo defeated: a fresh callback per render re-renders every row anyway

src/components/MetricsTable.jsx, lines 28, 29, 30, 31, 32, 33

  1. The memo is real, the benefit is not. Row is wrapped in React.memo, but the parent passes onSelect={(id) => setSelectedId(id)}, a brand-new function on every render. memo compares props by reference, sees a changed onSelect, and re-renders every row.
  2. Typing is the worst case. Each keystroke updates query, re-renders the parent, and with memo defeated, every visible row re-renders its Sparkline, the exact expensive work the memo was added to avoid.
  3. The fix is smaller than the bug. setSelectedId from useState already has a stable identity: pass it directly. (For anything less convenient, useCallback does the same job.)

The fix: onSelect={setSelectedId}, same behavior, stable identity, memo works.

The fix

{visible.map((metric) => (
  <Row
    key={metric.id}
    metric={metric}
    onSelect={setSelectedId}
  />
))}

Why it matters

A memo/PureComponent in the tree → check every prop at the call site: "is anything created fresh in this render?" An inline lambda or object literal silently cancels the whole optimization.

Real-world stakes: the dashboard is smooth in the demo with 40 metrics, and typing feels like wading through glue for the customer with 5,000. Nothing errors, profiling is the only witness. AI assistants generate this pairing constantly, they add React.memo as a best practice and inline callbacks as a habit, and the two cancel out.

Read more: react memo inline function prop, useCallback stable reference, react re-render every keystroke list

The verdict

Request changes. memo defeated: a fresh callback per render re-renders every row anyway

Try a live one

React PRs you have not seen the answer to.

Leave inline comments and a verdict, then see what you caught. Free, no signup.

Review a React PR →

How to review React code · All code review examples