fix(feed): abort the in-flight load-more when the feed filter changes - #110
fix(feed): abort the in-flight load-more when the feed filter changes#110lopatnov wants to merge 3 commits into
Conversation
useFeedPage hand-rolled its own cursor pagination instead of going through useCursorPaginatedList, and never received the shared-AbortController fix that 7060fef applied to that hook. Its load-more path called loadFeed(cursor) with no signal at all, so nothing could cancel it: - switching the source filter or the unread toggle while a load-more was in flight let the stale page resolve into appendItems(), splicing articles of the previous filter into the list that replaced them; - the same response overwrote nextCursor, so every subsequent page was pulled from the wrong filter's cursor; - its finally block ran `if (!signal?.aborted)` against an undefined signal, which is always falsy, so it cleared the loading flag the replacement request had just set — dropping the skeleton and re-arming load-more early. Extract the "one request in flight, starting a new one aborts the old" rule that useCursorPaginatedList already implemented correctly into useLatestRequest and use it from both hooks, so the invariant cannot drift out of sync in one of them again. loadFeed's signal is now required rather than optional, which is what let an unabortable call be written in the first place. No behaviour change for useCursorPaginatedList — same controller discipline, just relocated. Needs test coverage (follow-up tester pass): switching the unread filter and the ?sub= source filter while a load-more is in flight must leave neither a stale item nor a stale nextCursor in the feed store; FeedPage.test.tsx currently mocks useIntersectionLoader out entirely, so the load-more path has no coverage at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg
Verified the test fails against the pre-fix useFeedPage.ts (nextCursor gets overwritten with the stale load-more response's cursor), and passes against the fix — confirms both that the bug was real and that the fix in the previous commit resolves it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hnoh6YfJwp63szeP8ZFqVg
|
Warning Review limit reached
Next review available in: 51 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
2 participants
Summary
useFeedPage'shandleLoadMore()calledloadFeed(cursor)with noAbortSignal(the parameter was optional and defaulted toundefined), while the filter-change effect used its own separateAbortController. If a user triggered "load more" and then changed the source/unread filter before the load-more response came back, there was no way to cancel the stale request. When it resolved late, it appended its (now-stale) items onto the list the filter change had already replaced, and overwrotenextCursorwith a cursor belonging to the wrong filter. Thefinallyblock'sif (!signal?.aborted) setLoading(false)was also unconditionally true for an unabortable call, so it could clear the replacement request's loading flag too.useCursorPaginatedListalready had correctly into a new shared hook,useLatestRequest.ts(tracks one "latest request"AbortController, aborting whatever's in flight when a new one starts). BothuseFeedPageanduseCursorPaginatedListnow route through it, so the two hooks can't drift out of sync again.loadFeed'ssignalparameter is now required, not optional, so a future caller can't skip the discipline the wayhandleLoadMoredid.Found by
architectduring the periodic/maintainrefactor-analysis pass (2026-08-15), off-signal fromrepo-scout's file-size/component-count report — a genuine SOLID/duplication read of the hooks, not a line-count trigger. Independently verified before merging: reproduced the bug against the pre-fix code (confirmed the new test fails there —nextCursorends up as the stale cursor) and confirmed it passes against the fix.Related issue
Part of the periodic
/maintaincycle (2026-08-15) — step 3 (refactor analysis: real bug found along the way).Type of change
Checklist
dotnet build --configuration Releasepasses with 0 errors (unaffected by this branch)cd src/pressmark-web && npm run buildpasses with 0 TypeScript errorsFeedPage — load-more race condition, verified it fails against the pre-fix hook and passes against the fixt('ns:key')— no strings touchedNotes for reviewers
No design ambiguity —
useCursorPaginatedListalready had the correct pattern (sharedAbortControlleracrossloadMoreand the reset effect);useFeedPagehad drifted from it by hand-rolling its own version that forgot to wire load-more through the abort logic at all. The fix is a straight extraction of the already-correct pattern into a shared hook, applied uniformly to both.Generated by Claude Code