fix: prevent eager requests on Thread component mount - #3282
fix: prevent eager requests on Thread component mount#3282MartinCupela wants to merge 2 commits into
Conversation
A thread does not exist server-side until its parent message has a reply, so opening one to write the first reply produced a `GET /threads/:id` that could only 404 — `Thread.reload()` already swallowed exactly that answer and returned without state, so the request bought nothing. The reply list then asked for a page of its own: an empty list sits within the scroll threshold of both its top and its bottom, so the infinite scroller's mount-time observation requested one in each direction. Three guards, each reading the parent message's reply count: - `Thread` skips its initial load, and the stale reload, while `replyCount` is 0 - `useCanPaginateReplies` disarms the message list's scroll-driven loads inside a thread with no replies (channel lists are unaffected) - the vite example resolves a `thread:` deep link from the parent message — reusing a listed thread, else the message store, else `GET /messages/:id`, which answers for a reply-less message — instead of querying the thread None of them are sticky: `replyCount` projects the parent's `reply_count`, which the server keeps current over the WS, so the first reply arms all three without a remount. `isStateStale` is only cleared by a successful reload, so a thread that goes stale while empty still catches up the moment it has something to catch up on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Size Change: +3.91 kB (+0.48%) Total Size: 825 kB 📦 View Changed
ℹ️ View Unchanged
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-v15 #3282 +/- ##
==============================================
Coverage ? 84.92%
==============================================
Files ? 521
Lines ? 15468
Branches ? 4951
==============================================
Hits ? 13136
Misses ? 2332
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
depends on GetStream/stream-chat-js#1860
Description
Opening a thread made requests it didn't need — in three different ways, all with the same shape: the app asked the server for replies it either already had or knew didn't exist.
1. A thread with no replies was queried and 404'd
A thread doesn't exist server-side until its parent message has a reply. Opening one to write the first reply produced
GET /threads/:id, which answers404. The SDK already swallowed that answer —Thread.reload()treats a 404 withreplyCount === 0as "never created" and returns without touching state — so the request could only ever fail.Threadnow skips its initial load, and its stale reload, while the parent message reports no replies.2. Opening a thread that has replies made two requests for the same page
GET /threads/:id?reply_limit=50(the thread's own load, which also hydrates participants, read state and the watch) andGET /messages/:id/replies?limit=50(the message list) fetched the same first page concurrently.The message list asks for more replies whenever it is scrolled near either end — and a list that is empty, or barely taller than its container, is near both ends at once. With nothing loaded, its paginator can't refuse: "more above/below" is optimistically true until something has been fetched. The new
useCanPaginateReplieshook supplies the missing information from the thread itself.3. Sending the first reply fetched it back
The guard turning from off to on changes the two
loadNext*callbacks passed toInfiniteScrollPaginator, which rebuilds its memoized scroll handler and re-observes the list. On a one-item list that immediately asks for a page — for the reply just sent.The rule is now two ordered checks: with nothing loaded, the first page belongs to the thread's own load; otherwise arm only when the parent reports more replies than the list is showing.
4. The vite example queried threads from URL parameters
Restoring a
thread:token from?workspace=calledgetThreadAndHydrate, so a deep link to a reply-less thread 404'd on load. It now resolves the thread from the parent message — reusing a listed thread, else the message store, elseGET /messages/:id, which answers for any message regardless of replies.Not sticky
None of the guards latch.
replyCountis a projection of the parent message'sreply_count, which the server keeps current over the websocket, so a reply from anyone arms all of them without a remount.isStateStaleis only cleared by a successful reload, so a thread that goes stale while empty still catches up the moment it has something to catch up on.Dependency
Requires GetStream/stream-chat-js#1860, and a release containing it.
Point 2 suppresses
GET /messages/:id/replies, which — until that fix — was the only request actually populating a locally-opened thread:Thread.reload()fetched the replies andmergeNewestPagediscarded them, because it refuses to seed a window that has never held one. Merging this alone leaves the thread panel empty.Tests
Thread.test.tsx— no load with no replies; deferred until the count reports one; a stale thread defers the same way and catches up.useCanPaginateReplies.test.tsx— the rule in isolation, including the two states that produced the extra requests: nothing loaded on a thread that has replies, and one reply loaded with a count of one.Full suite passes.
Verified in the app
With the LLC fix in place, opening a thread with replies now makes one request and renders the replies; opening a reply-less thread makes none; sending the first reply makes none beyond the send itself.