Skip to content

Commit b88be5a

Browse files
fix: search empty state never renders due to stale fetch race condition (#144) (#146)
Co-authored-by: Ona <no-reply@ona.com>
1 parent e229388 commit b88be5a

2 files changed

Lines changed: 97 additions & 13 deletions

File tree

src/components/sidebar/page-search.test.tsx

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,10 @@ describe("PageSearch", () => {
7979
await vi.advanceTimersByTimeAsync(350);
8080
});
8181

82-
// The fetch should have been called
82+
// The fetch should have been called with the query and an abort signal
8383
expect(fetchMock).toHaveBeenCalledWith(
84-
expect.stringContaining("zzzyyyxxxnonexistent999")
84+
expect.stringContaining("zzzyyyxxxnonexistent999"),
85+
expect.objectContaining({ signal: expect.any(AbortSignal) })
8586
);
8687

8788
// The empty state message should be visible
@@ -144,6 +145,72 @@ describe("PageSearch", () => {
144145
).toBeInTheDocument();
145146
});
146147

148+
it("aborts stale fetch when query changes rapidly", async () => {
149+
// Track abort signals to verify cancellation
150+
const signals: AbortSignal[] = [];
151+
fetchMock.mockImplementation((_url: string | URL | Request, init?: RequestInit) => {
152+
if (init?.signal) signals.push(init.signal);
153+
return new Promise<Response>((resolve) => {
154+
// Resolve after a delay, but only if not aborted
155+
setTimeout(() => {
156+
resolve(new Response(JSON.stringify({ results: [] }), {
157+
status: 200,
158+
headers: { "Content-Type": "application/json" },
159+
}));
160+
}, 100);
161+
});
162+
});
163+
164+
const user = userEvent.setup({
165+
advanceTimers: vi.advanceTimersByTime,
166+
});
167+
168+
render(<PageSearch />);
169+
170+
// Wait for workspace resolution
171+
await act(async () => {
172+
await vi.advanceTimersByTimeAsync(50);
173+
});
174+
175+
const input = screen.getByRole("combobox", { name: /search pages/i });
176+
await user.click(input);
177+
await user.type(input, "first");
178+
179+
// Advance past debounce to trigger first fetch
180+
await act(async () => {
181+
await vi.advanceTimersByTimeAsync(350);
182+
});
183+
184+
// First fetch should be in-flight
185+
expect(signals.length).toBe(1);
186+
expect(signals[0].aborted).toBe(false);
187+
188+
// Type a new query before first fetch resolves
189+
await user.clear(input);
190+
await user.type(input, "second");
191+
192+
// The first signal should now be aborted
193+
expect(signals[0].aborted).toBe(true);
194+
195+
// Advance past debounce for second query
196+
await act(async () => {
197+
await vi.advanceTimersByTimeAsync(350);
198+
});
199+
200+
// Second fetch should have been called
201+
expect(signals.length).toBe(2);
202+
203+
// Let second fetch resolve
204+
await act(async () => {
205+
await vi.advanceTimersByTimeAsync(150);
206+
});
207+
208+
// Should show empty state (second query returned no results)
209+
expect(
210+
screen.getByText("No pages match your search")
211+
).toBeInTheDocument();
212+
});
213+
147214
it("shows skeletons while workspace is resolving even after search completes", async () => {
148215
// Make workspace resolution hang
149216
let resolveWorkspace: (value: unknown) => void;

src/components/sidebar/page-search.tsx

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export function PageSearch() {
3232
const containerRef = useRef<HTMLDivElement>(null);
3333
const inputRef = useRef<HTMLInputElement>(null);
3434
const debounceRef = useRef<ReturnType<typeof setTimeout> | null>(null);
35+
const abortRef = useRef<AbortController | null>(null);
3536

3637
// Resolve workspace slug to ID
3738
useEffect(() => {
@@ -66,44 +67,55 @@ export function PageSearch() {
6667
}, [params.workspaceSlug]);
6768

6869
const search = useCallback(
69-
async (q: string) => {
70+
async (q: string, signal: AbortSignal) => {
7071
if (!q.trim() || !workspaceId) {
71-
setResults([]);
72-
setLoading(false);
72+
if (!signal.aborted) {
73+
setResults([]);
74+
setLoading(false);
75+
}
7376
return;
7477
}
7578

7679
try {
7780
const response = await fetch(
78-
`/api/search?q=${encodeURIComponent(q.trim())}&workspace_id=${encodeURIComponent(workspaceId)}`
81+
`/api/search?q=${encodeURIComponent(q.trim())}&workspace_id=${encodeURIComponent(workspaceId)}`,
82+
{ signal }
7983
);
84+
if (signal.aborted) return;
8085
if (response.ok) {
8186
const data = (await response.json()) as { results: SearchResult[] };
87+
if (signal.aborted) return;
8288
setResults(data.results);
8389
setSelectedIndex(0);
8490
} else {
8591
setResults([]);
8692
}
8793
} catch (error) {
94+
if (signal.aborted) return;
8895
Sentry.captureException(error);
8996
setResults([]);
9097
} finally {
91-
setLoading(false);
92-
setSearched(true);
98+
if (!signal.aborted) {
99+
setLoading(false);
100+
setSearched(true);
101+
}
93102
}
94103
},
95104
[workspaceId]
96105
);
97106

98-
// Debounced search (300ms). The timer always fires; the search callback
99-
// handles the case where workspaceId is not yet available by clearing
100-
// loading. The rendering layer uses workspaceResolved to decide whether
101-
// to show skeletons or the empty-state message.
107+
// Debounced search with abort support. Cancels any in-flight fetch when
108+
// the query or search callback changes, preventing stale responses from
109+
// overwriting the current state.
102110
useEffect(() => {
103111
if (debounceRef.current) {
104112
clearTimeout(debounceRef.current);
105113
debounceRef.current = null;
106114
}
115+
if (abortRef.current) {
116+
abortRef.current.abort();
117+
abortRef.current = null;
118+
}
107119

108120
if (!query.trim()) {
109121
setResults([]);
@@ -113,16 +125,21 @@ export function PageSearch() {
113125
}
114126

115127
setLoading(true);
128+
setSearched(false);
129+
const controller = new AbortController();
130+
abortRef.current = controller;
131+
116132
debounceRef.current = setTimeout(() => {
117133
debounceRef.current = null;
118-
search(query);
134+
search(query, controller.signal);
119135
}, 300);
120136

121137
return () => {
122138
if (debounceRef.current) {
123139
clearTimeout(debounceRef.current);
124140
debounceRef.current = null;
125141
}
142+
controller.abort();
126143
};
127144
}, [query, search]);
128145

0 commit comments

Comments
 (0)