fix: replace abort-signal staleness check with generation counter in search (#144)#151
Conversation
…search (#144) The search empty state ("No pages match your search") never rendered because the loading state could get stuck at true. The previous fix used AbortSignal.aborted in the finally block to decide whether to clear loading. This created a microtask ordering bug: when the debounce effect re-runs, it aborts the old controller and sets loading=true synchronously, but the aborted fetch finally block runs later as a microtask. If the abort races with fetch completion, the finally block could either skip clearing loading (leaving it stuck) or clear it at the wrong time. Replace the signal.aborted guard with a generation counter ref. Each search cycle increments the counter synchronously. Async callbacks only update state when their generation matches the current one. This is immune to microtask ordering because the counter is incremented in the same synchronous block that sets loading=true. Co-authored-by: Ona <no-reply@ona.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
✅ UI verification passed — design spec compliance confirmed. Static analysis: No visual changes in this PR. The diff replaces Visual verification (Playwright screenshots):
All design spec checks pass. |
|
✅ Post-merge verification passed. E2E suite: 41/42 passed. The 1 failure ( Ad-hoc smoke tests: All passed.
Notably: |
1 participant
Closes #144
What
The search empty state ("No pages match your search") never renders when searching for a nonsense string. Instead, skeleton loading placeholders show indefinitely. This bug persisted across three previous fix attempts (#126, #136, #146) because the root cause — a microtask ordering race between the AbortSignal and React state updates — was never addressed.
How
Replaced the
signal.abortedguard in the search callback'sfinallyblock with a generation counter ref (searchGenRef). Each search cycle increments the counter synchronously in the debounce effect. Async callbacks only update state when their captured generation matches the current ref value.The previous approach checked
signal.abortedin thefinallyblock to decide whether to clearloading. This failed because: when the debounce effect re-runs, it aborts the old controller and setsloading=truesynchronously, but the aborted fetch'sfinallyblock runs later as a microtask. If the abort races with fetch completion (signal not yet aborted whenfinallyruns), thefinallyblock could setloading=false, undoing the new cycle'sloading=true. The generation counter is immune to this because it is incremented synchronously before any microtask from the previous cycle can run.The
AbortControlleris retained for network efficiency (cancelling in-flight requests), but state management decisions are now driven entirely by the generation counter.Testing
pnpm test)pnpm test:e2e), including the previously failingsearch with no matches shows empty statepnpm lint && pnpm typecheckpass