2026-09-05 · 7 min read

React code review exercise: can an older search response win?

In 2022, someone filed a bug against Material UI's Autocomplete: during fast typing the input would randomly reset, flash "no results," then settle on the wrong suggestions. The root cause was async responses arriving out of order, with the stale one quietly overwriting the fresh one. jQuery UI's autocomplete had the same class of bug open for years. I have shipped this bug myself. Every frontend developer I know who has built a search box has shipped it at least once, because it is invisible on a fast office connection, impossible to reproduce on purpose unless you already know the cause, and the code that produces it looks completely correct.

This is a React code review exercise built around that exact pattern: a twelve-line search component where a useEffect fires a fetch on every keystroke and nothing cancels the previous request. It comes with the full worked solution, the fix, and a test that proves the race condition exists. If you are practicing JavaScript or React pull request review, or you want to build a reflex for catching async bugs in AI-generated code before they ship, this is the exercise.

The requirements

The PR description says:

Adds live user search. The component fetches /api/search whenever the query changes and renders the matching users. Tested locally with several queries and the list updates correctly on every keystroke.

The actual requirements are short:

Add a live search box.
As the user types, show the results that match the current query.
The displayed list must always correspond to the latest query the user typed.

Context from the ticket: this is used in the admin panel header. Production users often search from slow mobile networks.

The PR

ai-agent wants to merge · src/components/UserSearch.jsx +12

1+ function UserSearch({ query }) {

2+ const [results, setResults] = useState([]);

3+ useEffect(() => {

4+ fetch(`/api/search?q=${query}`)

5+ .then((res) => res.json())

6+ .then((data) => setResults(data));

7+ }, [query]);

8+ return (

9+ <ul>{results.map((u) => <li key={u.id}>{u.name}</li>)}</ul>

10+ );

11+ }

Eleven lines, new file, no deletions. The component takes query as a prop, fetches on every change, renders the result list. The author says it works locally. It does work locally. That is part of the problem.

Try it yourself

The same PR is waiting for your review, no spoilers.

Leave your inline comments and your verdict before you read the walkthrough below. Free, no signup.

Review this PR →

~ Walkthrough starts here ~

Twelve lines, what could go wrong

Start from what the code actually does when a user types "al" and then immediately types "alice". Two things happen in sequence: the effect fires for "al", starting a fetch, and then the effect fires again for "alice", starting a second fetch. Both are now in flight. Neither knows the other exists.

On the developer's office wifi, both responses come back in under 50 milliseconds, in order, and the UI looks perfect. But the requirements say this component lives in the admin panel header and production users search from slow mobile networks. On a congested 3G connection, the "al" request might take 800ms and the "alice" request might take 200ms (maybe it hits a cache, maybe the server is just faster for that particular query). Now "alice" resolves first and the UI correctly shows Alice. Then, 600 milliseconds later, "al" resolves and setResults(data) runs again, overwriting the screen with every user whose name contains "al": Alice, Alfred, Alejandra, Albert. The user is staring at the name "alice" in the search box and a list that does not match it. Requirement three is broken: the displayed list does not correspond to the latest query.

The ugly part is that no error fires. No test fails. No console warning appears. The component did exactly what the code says: it called setResults every time a response came back. The code just never bothered to ask whether the response was still relevant.

The comment you would leave

3+ useEffect(() => {

4+ fetch(`/api/search?q=${query}`)

5+ .then((res) => res.json())

6+ .then((data) => setResults(data));

7+ }, [query]);

Yyou commented

Race condition: when the user types fast (or the network is slow), an older response can land after a newer one and overwrite the current results. The effect has no cleanup, so nothing cancels or ignores the stale fetch. On a slow mobile network, typing "al" then "alice" can show "al" results under the "alice" query. Add a cleanup function that sets an ignore flag, or use an AbortController to cancel the previous request.

The verdict is Request changes. This is not a style nit or an edge case that might hypothetically matter someday. The requirements explicitly say the displayed list must always correspond to the latest query, and this code cannot guarantee that. On the networks the component is designed to run on, it will regularly show stale results with no indication that anything went wrong.

The fix

The simplest fix is the ignore flag pattern that the React docs recommend for exactly this situation. When the effect re-runs (because query changed), the cleanup function from the previous run sets ignore = true, and the old .then callback checks the flag before calling setResults:

3+ useEffect(() => {

4+ let ignore = false;

5+ fetch(`/api/search?q=${query}`)

6+ .then((res) => res.json())

7+ .then((data) => { if (!ignore) setResults(data); });

8+ return () => { ignore = true; };

9+ }, [query]);

That is three lines added to the original code. The closure captures ignore while it is false. If query changes before the fetch completes, the cleanup runs, ignore becomes true, and when the stale response finally arrives, setResults never fires. The new, current fetch has its own ignore variable that is still false, so it updates the UI normally.

You could also use an AbortController, which actually cancels the HTTP request rather than just ignoring its result:

3+ useEffect(() => {

4+ const ctrl = new AbortController();

5+ fetch(`/api/search?q=${query}`, { signal: ctrl.signal })

6+ .then((res) => res.json())

7+ .then((data) => setResults(data))

8+ .catch((e) => { if (e.name !== 'AbortError') throw e; });

9+ return () => ctrl.abort();

10+ }, [query]);

The AbortController version is slightly better in production because it tells the browser to drop the connection, saving bandwidth on mobile. Both versions solve the race condition. The ignore flag is the minimum viable fix; the abort is the production-grade one. Either is a valid request in review.

A test that actually catches it

The trick to testing a race condition is controlling the order responses resolve. You cannot prove the bug exists by firing one request and checking the result, because the bug only manifests when two requests overlap. Here is a minimal test that breaks on the original code and passes on the fix:

1+ let resolvers = {};

2+ global.fetch = (url) => new Promise((resolve) => {

3+ const q = new URL(url, 'http://x').searchParams.get('q');

4+ resolvers[q] = (data) =>

5+ resolve({ json: () => Promise.resolve(data) });

6+ });

8+ // Render with query="al", then re-render with query="alice"

9+ render(<UserSearch query="al" />);

10+ render(<UserSearch query="alice" />);

11+

12+ // Resolve "alice" FIRST, then "al" SECOND

13+ await act(() => resolvers['alice']([{ id: 1, name: 'Alice' }]));

14+ await act(() => resolvers['al']([

15+ { id: 1, name: 'Alice' }, { id: 2, name: 'Alfred' }

16+ ]));

17+

18+ // The UI should show only Alice, not Alice + Alfred

19+ expect(screen.getAllByRole('listitem')).toHaveLength(1);

20+ expect(screen.getByText('Alice')).toBeInTheDocument();

The key is line 13 and 14: the "alice" response resolves first, and then the "al" response resolves second, simulating the exact out-of-order arrival that happens on a slow network. On the original code, after both resolve, the screen shows both Alice and Alfred (the "al" results), because the last setResults call wins regardless of which query it belongs to. The assertion on line 19 fails: expected 1 item, got 2. On the fixed code, the "al" callback checks ignore, finds it true, and skips the update. Only Alice remains on screen. The test passes.

Notice what the test does not do: it does not fire a single request and check whether the right data appeared. That test passes on the buggy code too, because single requests always work. The only way to catch this bug is to put two requests in flight and resolve them in the wrong order.

The Monday morning version of this bug

The JSON says this component lives in the admin panel header, used for looking up users. Think about a customer support agent searching for "John Smith" to issue a refund. They type "john", pause, then type "john smith". If "john" is slow and "john smith" is fast, the screen briefly shows John Smith, then silently replaces it with every John in the database. The support agent clicks the first result, which is now John Martinez, and refunds the wrong customer. There is no error in the logs, no failed assertion, nothing but a confused customer calling back to ask why they got a mysterious refund and John Smith still waiting.

The reason this bug class survives code review so well is that it requires the reviewer to simulate time. The code, read line by line, is correct: it fetches, it parses, it updates state, it depends on the right variable. You have to hold two copies of the effect in your head, running concurrently, and ask yourself what happens when one finishes before the other in the wrong order. Most reviews do not do that. Most reviews read the code like a single thread of execution, top to bottom, and this code reads perfectly in that mode.

The reflex to build

Any time you see a fetch (or any async call) inside a useEffect, and the effect has no cleanup function, stop and ask: what happens when two of these are in flight at the same time? That single question catches this entire class of bug. It does not require you to be an expert on AbortController or to have memorized the React docs on data fetching. It just requires you to notice the missing cleanup and wonder what it is supposed to be cleaning up.

The pattern generalizes beyond search. Profile pages that fetch by user ID, dashboards that reload on tab change, chat rooms that load history by room ID: any component where a prop drives a fetch and the user can change that prop faster than the network can respond. The question is always the same: is there a guard against the stale response?