From 3cf839d65479dd5f3e680200b72fe801e439cfb9 Mon Sep 17 00:00:00 2001 From: martincupela Date: Wed, 9 Sep 2026 12:04:33 +0200 Subject: [PATCH 1/2] fix(Thread): stop querying a thread that has no replies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../vite/src/ChatLayout/WorkspaceUrlSync.tsx | 67 ++++++++++++--- src/components/MessageList/MessageList.tsx | 12 ++- .../MessageList/VirtualizedMessageList.tsx | 8 +- .../hooks/useCanPaginateReplies.ts | 30 +++++++ src/components/Thread/Thread.tsx | 20 ++++- .../Thread/__tests__/Thread.test.tsx | 84 ++++++++++++++++++- 6 files changed, 203 insertions(+), 18 deletions(-) create mode 100644 src/components/MessageList/hooks/useCanPaginateReplies.ts diff --git a/examples/vite/src/ChatLayout/WorkspaceUrlSync.tsx b/examples/vite/src/ChatLayout/WorkspaceUrlSync.tsx index 9ee8b08df..15f02ee07 100644 --- a/examples/vite/src/ChatLayout/WorkspaceUrlSync.tsx +++ b/examples/vite/src/ChatLayout/WorkspaceUrlSync.tsx @@ -10,7 +10,14 @@ import { useChatViewContext, useChatViewNavigation, } from 'stream-chat-react/slot-layout'; -import type { Channel, ChannelManager, StreamChat, Thread } from 'stream-chat'; +import { formatMessage, Thread as StreamThread } from 'stream-chat'; +import type { + Channel, + ChannelManager, + LocalMessage, + StreamChat, + Thread, +} from 'stream-chat'; /** * Full-workspace URL sync for the vite example. @@ -190,6 +197,23 @@ const resolveChannel = (client: StreamChat, cid: string): Channel | undefined => return type && id ? client.channel(type, id) : undefined; }; +/** + * One message by id, for a thread deep link whose parent is outside every loaded window (a link + * into an old thread, or a cold Back). `GET /messages/:id` answers for any message, including one + * with no replies — unlike the thread endpoint, which 404s until the first reply exists. + */ +const fetchMessage = async ( + client: StreamChat, + id: string, +): Promise => { + try { + const { message } = await client.getMessage({ id }); + return message ? formatMessage(message) : undefined; + } catch { + return undefined; + } +}; + const resolveBinding = async ( client: StreamChat, token: ParsedToken, @@ -210,17 +234,38 @@ const resolveBinding = async ( } case 'thread': { // Paginator-first: a thread the thread-list already holds is reused as-is — no round-trip. - // Only when it isn't loaded (deep-link straight to a thread past page 1, or a cold Back into a - // never-visited thread) do we fall back to fetching it by id. - const thread = - client.threads.threadsById[token.key] ?? - (await client - .getThreadAndHydrate(token.key, { watch: true }) - .catch(() => undefined)); - if (!thread) return undefined; + const listed = client.threads.threadsById[token.key]; + if (listed) { + return { + binding: { key: listed.id ?? undefined, kind: 'thread', source: listed }, + channel: listed.channel ?? undefined, + }; + } + + // Otherwise build the instance from its parent message rather than querying the thread. + // + // Two reasons not to call `getThreadAndHydrate` here. A thread does not exist server-side + // until its parent message has a reply, so restoring a link to a reply-less thread would + // answer 404 — and the query is redundant even for a real thread, because `` loads + // its own replies once the parent reports some. Deciding that is the component's job; this + // resolver only has to produce the instance to bind. + const parentMessage = + client.messageStore.get(token.key) ?? (await fetchMessage(client, token.key)); + if (!parentMessage?.cid) return undefined; + + const channel = resolveChannel(client, parentMessage.cid); + if (!channel) return undefined; + // Same watch the bound `` would issue (see the channel case) — moved earlier so the + // thread's channel config, members and read state are loaded when the panel renders. + if (!channel.initialized) await channel.watch().catch(() => undefined); + return { - binding: { key: thread.id ?? undefined, kind: 'thread', source: thread }, - channel: thread.channel ?? undefined, + binding: { + key: token.key, + kind: 'thread', + source: new StreamThread({ channel, client, parentMessage }), + }, + channel, }; } case 'userProfile': diff --git a/src/components/MessageList/MessageList.tsx b/src/components/MessageList/MessageList.tsx index 136904c74..54bbc2016 100644 --- a/src/components/MessageList/MessageList.tsx +++ b/src/components/MessageList/MessageList.tsx @@ -50,6 +50,7 @@ import type { InfiniteScrollPaginatorProps } from '../InfiniteScrollPaginator/In import { InfiniteScrollPaginator } from '../InfiniteScrollPaginator/InfiniteScrollPaginator'; import { useMessagePaginator } from '../../hooks'; import { ScrollToLatestMessageButton } from './ScrollToLatestMessageButton'; +import { useCanPaginateReplies } from './hooks/useCanPaginateReplies'; type MessageListWithContextProps = MessageListProps; @@ -233,6 +234,9 @@ const MessageListWithContext = (props: MessageListWithContextProps) => { const messageListClass = customClasses?.messageList || 'str-chat__message-list'; + // An empty thread would otherwise ask for a page at both ends the moment the scroller mounts. + const canPaginateReplies = useCanPaginateReplies(); + const loadOlderMessages = React.useCallback(async () => { if (loadingOlderRef.current) return; loadingOlderRef.current = true; @@ -385,8 +389,12 @@ const MessageListWithContext = (props: MessageListWithContextProps) => { className='str-chat__message-list-scroll' data-testid='reverse-infinite-scroll' element={internalListElement} - loadNextOnScrollToBottom={messagePaginator.toHead} - loadNextOnScrollToTop={loadOlderMessages} + loadNextOnScrollToBottom={ + canPaginateReplies ? messagePaginator.toHead : undefined + } + loadNextOnScrollToTop={ + canPaginateReplies ? loadOlderMessages : undefined + } onScroll={onScroll} ref={setListElement} threshold={loadMoreScrollThreshold} diff --git a/src/components/MessageList/VirtualizedMessageList.tsx b/src/components/MessageList/VirtualizedMessageList.tsx index ded4d9044..9ae705ed8 100644 --- a/src/components/MessageList/VirtualizedMessageList.tsx +++ b/src/components/MessageList/VirtualizedMessageList.tsx @@ -73,6 +73,7 @@ import type { UserResponse, } from 'stream-chat'; import type { UnknownType } from '../../types/types'; +import { useCanPaginateReplies } from './hooks/useCanPaginateReplies'; import { useStableId } from '../UtilityComponents/useStableId'; import { useLastDeliveredData } from './hooks/useLastDeliveredData'; import { useLastOwnMessage } from './hooks/useLastOwnMessage'; @@ -490,18 +491,23 @@ const VirtualizedMessageListWithContext = ( [], ); + const canPaginateReplies = useCanPaginateReplies(); + const atBottomStateChange = (isAtBottom: boolean) => { atBottom.current = isAtBottom; setIsMessageListScrolledToBottom(isAtBottom); if (isAtBottom) { - messagePaginator.toHead(); + // An empty thread is at both ends at once, so Virtuoso reports both on mount — see + // `useCanPaginateReplies` for why that must not become a request. + if (canPaginateReplies) messagePaginator.toHead(); // loadMoreNewer?.(messageLimit); setNewMessagesNotification?.(false); } }; const atTopStateChange = (isAtTop: boolean) => { if (isAtTop) { + if (!canPaginateReplies) return; if (loadingOlderRef.current) return; loadingOlderRef.current = true; setSuppressAutoscrollWhileLoadingOlder(true); diff --git a/src/components/MessageList/hooks/useCanPaginateReplies.ts b/src/components/MessageList/hooks/useCanPaginateReplies.ts new file mode 100644 index 000000000..dab9a2c57 --- /dev/null +++ b/src/components/MessageList/hooks/useCanPaginateReplies.ts @@ -0,0 +1,30 @@ +import { useThreadContext } from '../../Threads'; +import { useStateStore } from '../../../store'; + +import type { ThreadState } from 'stream-chat'; + +const selector = ({ replyCount }: ThreadState) => ({ replyCount }); + +/** + * Whether the list this hook is rendered in has replies worth paginating. + * + * Always `true` outside a thread — a channel list paginates regardless. + * + * Inside a thread it follows the parent message's reply count, and the reason is the shape of an + * empty list: it sits within the scroll threshold of BOTH its top and its bottom, so the infinite + * scroller's mount-time observation asks for a page in each direction. On a thread with no replies + * those are requests for messages that cannot exist — and until the first reply the thread itself + * does not exist server-side. The reply paginator can't tell: it has no loaded window, so "more + * headward/tailward" is optimistically true, which is correct in general and wrong here. The count + * is the missing piece, and it lives on the thread. + * + * Not sticky: `replyCount` projects the parent message's `reply_count`, which the server keeps + * current over the WS, so pagination arms itself the moment a reply exists. + */ +export const useCanPaginateReplies = (): boolean => { + const thread = useThreadContext(); + const { replyCount } = useStateStore(thread?.state, selector) ?? {}; + + if (!thread) return true; + return (replyCount ?? 0) > 0; +}; diff --git a/src/components/Thread/Thread.tsx b/src/components/Thread/Thread.tsx index 81017a058..738db286f 100644 --- a/src/components/Thread/Thread.tsx +++ b/src/components/Thread/Thread.tsx @@ -72,6 +72,7 @@ export const Thread = (props: ThreadProps) => { const selector = (nextValue: ThreadState) => ({ isStateStale: nextValue.isStateStale, parentMessage: nextValue.parentMessage, + replyCount: nextValue.replyCount, }); const messagePaginatorSelector = ({ @@ -107,7 +108,7 @@ const ThreadInner = (props: ThreadProps & { key: string }) => { const { ThreadHead = DefaultThreadHead, ThreadHeader = DefaultThreadHeader } = useComponentContext(); - const { isStateStale, parentMessage } = + const { isStateStale, parentMessage, replyCount } = useStateStore(threadInstance?.state, selector) ?? {}; const threadPaginatorState = useStateStore( threadInstance?.messagePaginator?.state, @@ -137,13 +138,26 @@ const ThreadInner = (props: ThreadProps & { key: string }) => { // which the virtualized list applies to its own subtree), so nothing is resolved here. const ThreadMessageList = virtualized ? VirtualizedMessageList : MessageList; + // A thread exists server-side only once its parent has a reply, so loading one with `replyCount` + // 0 is a request that can only 404 — `Thread.reload()` swallows exactly that and returns without + // state, so it buys nothing. + // + // This defers the load, it does not cancel it. `isStateStale` is only cleared by a successful + // reload (`thread.ts:589`), so while a thread stays stale, `replyCount` flipping to > 0 re-runs + // the effect below and the catch-up happens then. That covers the `user.watching.stop` case: we + // learn about replies missed while unwatched as soon as the parent message copy is refreshed, + // which is the same moment every other reply-count affordance in the UI learns about them. + const hasServerSideThread = (replyCount ?? 0) > 0; + useEffect(() => { if (!threadInstance) return; if (isThreadManaged) return; + if (!hasServerSideThread) return; if (threadPaginatorState?.items !== undefined || threadPaginatorState?.isLoading) return; void threadInstance.reload(); }, [ + hasServerSideThread, isThreadManaged, threadInstance, threadPaginatorState?.isLoading, @@ -151,10 +165,10 @@ const ThreadInner = (props: ThreadProps & { key: string }) => { ]); useEffect(() => { - if (threadInstance && isStateStale) { + if (threadInstance && isStateStale && hasServerSideThread) { void threadInstance.reload(); } - }, [isStateStale, threadInstance]); + }, [hasServerSideThread, isStateStale, threadInstance]); useEffect(() => { if (!threadInstance || isThreadManaged) return; diff --git a/src/components/Thread/__tests__/Thread.test.tsx b/src/components/Thread/__tests__/Thread.test.tsx index 5843dc933..2dbbdb7d9 100644 --- a/src/components/Thread/__tests__/Thread.test.tsx +++ b/src/components/Thread/__tests__/Thread.test.tsx @@ -71,10 +71,15 @@ const makeThread = ( items?: LocalMessage[] | undefined; parentMessage?: LocalMessage; replies?: boolean; + replyCount?: number; } = {}, ) => { const { isLoading = false, isStateStale = false, replies = true } = opts; const parent = opts.parentMessage ?? parentMessage; + // `ThreadState.replyCount` is a projection of the parent message's `reply_count` (the SDK keeps + // the two in sync through the message store), so derive it here instead of letting callers set + // the two independently. + const replyCount = opts.replyCount ?? parent.reply_count ?? 0; // Distinguish "not provided" (default to loaded replies) from an explicit `undefined` // (replies not fetched yet) — a destructuring default cannot tell them apart. const items = 'items' in opts ? opts.items : [reply1, reply2]; @@ -97,7 +102,7 @@ const makeThread = ( }, reload, state: new StateStore( - fromPartial({ isStateStale, parentMessage: parent }), + fromPartial({ isStateStale, parentMessage: parent, replyCount }), ), }); return { deactivate, reload, thread }; @@ -296,6 +301,83 @@ describe('Thread', () => { expect(reload).toHaveBeenCalledTimes(1); }); + it('should not reload a thread whose parent message has no replies yet', () => { + // The thread does not exist server-side until its first reply, so `GET /threads/:id` can only + // 404 here — opening a reply-less message to write the first reply must not query. + const { reload, thread } = makeThread({ + items: undefined, + parentMessage: generateMessage({ + id: 'never-created-parent', + reply_count: 0, + user: alice, + }), + }); + renderComponent({ threadInstance: thread }); + + expect(reload).not.toHaveBeenCalled(); + }); + + it('should reload once the parent message reports its first reply', () => { + // The skip is self-healing: `replyCount` follows the parent message, so the thread loads as + // soon as it exists server-side — without remounting the component. + const { reload, thread } = makeThread({ + items: undefined, + parentMessage: generateMessage({ + id: 'first-reply-parent', + reply_count: 0, + user: alice, + }), + }); + renderComponent({ threadInstance: thread }); + expect(reload).not.toHaveBeenCalled(); + + act(() => { + thread.state.partialNext({ replyCount: 1 }); + }); + + expect(reload).toHaveBeenCalledTimes(1); + }); + + it('should defer a stale reload until the thread reports a reply', () => { + // Reopening a closed thread reuses the cached instance, which `unregisterSubscriptions` left + // stale — for a thread that was never created that reload can only 404. The guard defers it: + // `isStateStale` stays true until a reload succeeds, so the catch-up runs as soon as the + // parent message reports a reply. + const { reload, thread } = makeThread({ + isStateStale: true, + // `[]`, not `undefined`: reopening runs on a disposed paginator, which is what makes the + // stale effect the only one that can still load this thread. + items: [], + parentMessage: generateMessage({ + id: 'stale-never-created-parent', + reply_count: 0, + user: alice, + }), + }); + renderComponent({ threadInstance: thread }); + expect(reload).not.toHaveBeenCalled(); + + act(() => { + thread.state.partialNext({ replyCount: 3 }); + }); + + expect(reload).toHaveBeenCalledTimes(1); + }); + + it('should reload a stale thread that has replies', () => { + const { reload, thread } = makeThread({ + isStateStale: true, + parentMessage: generateMessage({ + id: 'stale-parent', + reply_count: 2, + user: alice, + }), + }); + renderComponent({ threadInstance: thread }); + + expect(reload).toHaveBeenCalledTimes(1); + }); + it('should render null if replies is disabled', () => { const { thread } = makeThread({ replies: false }); const { container } = renderComponent({ threadInstance: thread }); From 20a8e10be522fcb0afdda9f4abf8a71a36ee5f7e Mon Sep 17 00:00:00 2001 From: martincupela Date: Thu, 10 Sep 2026 14:02:45 +0200 Subject: [PATCH 2/2] fix(Thread): stop fetching thread replies the app already has --- .../__tests__/useCanPaginateReplies.test.tsx | 95 +++++++++++++++++++ .../hooks/useCanPaginateReplies.ts | 49 +++++++--- 2 files changed, 131 insertions(+), 13 deletions(-) create mode 100644 src/components/MessageList/hooks/__tests__/useCanPaginateReplies.test.tsx diff --git a/src/components/MessageList/hooks/__tests__/useCanPaginateReplies.test.tsx b/src/components/MessageList/hooks/__tests__/useCanPaginateReplies.test.tsx new file mode 100644 index 000000000..90cd78c0d --- /dev/null +++ b/src/components/MessageList/hooks/__tests__/useCanPaginateReplies.test.tsx @@ -0,0 +1,95 @@ +import React from 'react'; +import { renderHook } from '@testing-library/react'; +import { fromPartial } from '@total-typescript/shoehorn'; +import { StateStore } from 'stream-chat'; +import { describe, expect, it } from 'vitest'; + +import type { PropsWithChildren } from 'react'; +import type { LocalMessage, Thread as StreamThread, ThreadState } from 'stream-chat'; + +import { ThreadProvider } from '../../../Threads'; +import { useCanPaginateReplies } from '../useCanPaginateReplies'; +import { generateMessage } from '../../../../mock-builders'; + +const makeThread = ({ + items, + replyCount, +}: { + items?: LocalMessage[]; + replyCount: number; +}) => + fromPartial({ + messagePaginator: { + state: new StateStore<{ items: LocalMessage[] | undefined }>({ items }), + }, + state: new StateStore(fromPartial({ replyCount })), + }); + +const renderWithThread = (thread?: StreamThread) => + renderHook(() => useCanPaginateReplies(), { + wrapper: ({ children }: PropsWithChildren) => ( + {children} + ), + }); + +describe('useCanPaginateReplies', () => { + it('allows pagination outside a thread', () => { + const { result } = renderWithThread(undefined); + + expect(result.current).toBe(true); + }); + + it('refuses on a thread with no replies', () => { + // Nothing to fetch, and until the first reply the thread does not exist server-side. + const { result } = renderWithThread(makeThread({ items: undefined, replyCount: 0 })); + + expect(result.current).toBe(false); + }); + + it('refuses when the list already holds every reply', () => { + // The state right after the first reply is sent: it is ingested locally and the parent's count + // has caught up, so arming the scroller would fetch a page that is already in hand. + const { result } = renderWithThread( + makeThread({ items: [generateMessage() as LocalMessage], replyCount: 1 }), + ); + + expect(result.current).toBe(false); + }); + + it('allows pagination when the parent reports replies the list does not hold', () => { + const { result } = renderWithThread( + makeThread({ items: [generateMessage() as LocalMessage], replyCount: 5 }), + ); + + expect(result.current).toBe(true); + }); + + it('refuses while nothing is loaded, even on a thread that has replies', () => { + // The first page belongs to `Thread.reload()` (`GET /threads/:id`, which also hydrates and + // watches). Arming here would fetch the same page again as `GET /messages/:id/replies`. + const { result } = renderWithThread(makeThread({ items: undefined, replyCount: 2 })); + + expect(result.current).toBe(false); + }); + + it('refuses on a reopened thread whose paginator was disposed', () => { + // `unregisterSubscriptions` leaves `items` as `[]` rather than `undefined`; the stale reload + // provides the first page, so the scroller still must not race it. + const { result } = renderWithThread(makeThread({ items: [], replyCount: 2 })); + + expect(result.current).toBe(false); + }); + + it('arms once a page is loaded and the parent reports more', () => { + const thread = makeThread({ items: undefined, replyCount: 120 }); + const { rerender, result } = renderWithThread(thread); + expect(result.current).toBe(false); + + thread.messagePaginator.state.partialNext({ + items: Array.from({ length: 50 }, () => generateMessage() as LocalMessage), + }); + rerender(); + + expect(result.current).toBe(true); + }); +}); diff --git a/src/components/MessageList/hooks/useCanPaginateReplies.ts b/src/components/MessageList/hooks/useCanPaginateReplies.ts index dab9a2c57..c1dc1e9d2 100644 --- a/src/components/MessageList/hooks/useCanPaginateReplies.ts +++ b/src/components/MessageList/hooks/useCanPaginateReplies.ts @@ -1,30 +1,53 @@ import { useThreadContext } from '../../Threads'; import { useStateStore } from '../../../store'; -import type { ThreadState } from 'stream-chat'; +import type { LocalMessage, ThreadState } from 'stream-chat'; -const selector = ({ replyCount }: ThreadState) => ({ replyCount }); +const threadSelector = ({ replyCount }: ThreadState) => ({ replyCount }); + +const paginatorSelector = ({ items }: { items: LocalMessage[] | undefined }) => ({ + loadedCount: items?.length ?? 0, +}); /** - * Whether the list this hook is rendered in has replies worth paginating. + * Whether the list this hook is rendered in has replies left to fetch by scrolling. * * Always `true` outside a thread — a channel list paginates regardless. * - * Inside a thread it follows the parent message's reply count, and the reason is the shape of an - * empty list: it sits within the scroll threshold of BOTH its top and its bottom, so the infinite - * scroller's mount-time observation asks for a page in each direction. On a thread with no replies - * those are requests for messages that cannot exist — and until the first reply the thread itself - * does not exist server-side. The reply paginator can't tell: it has no loaded window, so "more - * headward/tailward" is optimistically true, which is correct in general and wrong here. The count - * is the missing piece, and it lives on the thread. + * It exists because of the shape of a short list: it sits within the scroll threshold of BOTH its + * top and its bottom, so the infinite scroller asks for a page in each direction as soon as it + * observes its own size. The reply paginator cannot refuse while it has never queried — "more + * headward/tailward" is optimistically true then, which is right in general and wrong here. The + * counts are the missing piece, and they live on the thread. + * + * Two rules, in order: + * + * - **Nothing loaded yet → no.** The first page is the thread's own job: `Thread.reload()` fetches + * it through `GET /threads/:id`, which hydrates participants, read state and a watch alongside + * the replies. Arming here would ask for the same page again through + * `GET /messages/:id/replies`. A thread with no replies at all is the same rule — there is + * nothing to load, and until the first reply the thread does not exist server-side. + * - **Otherwise, only when the parent reports replies the list does not hold.** Which also covers + * the moment the first reply is sent: it is ingested locally and the count catches up, so there + * is nothing left to ask for. + * + * The count is the raw window length, so a message the server never acknowledged (a failed send, + * say) counts toward it. That can only under-arm, and only for a window that is BOTH partially + * loaded and padded with enough local-only messages to reach `reply_count` — narrow enough not to + * pay for a per-emission scan of the list. * * Not sticky: `replyCount` projects the parent message's `reply_count`, which the server keeps - * current over the WS, so pagination arms itself the moment a reply exists. + * current over the WS, and `loadedCount` follows the paginator, so both re-evaluate on their own. */ export const useCanPaginateReplies = (): boolean => { const thread = useThreadContext(); - const { replyCount } = useStateStore(thread?.state, selector) ?? {}; + const { replyCount } = useStateStore(thread?.state, threadSelector) ?? {}; + const { loadedCount } = useStateStore( + thread?.messagePaginator?.state, + paginatorSelector, + ) ?? { loadedCount: 0 }; if (!thread) return true; - return (replyCount ?? 0) > 0; + if (loadedCount === 0) return false; + return (replyCount ?? 0) > loadedCount; };