diff --git a/CLAUDE.md b/CLAUDE.md index 45a3733f4..062a74a41 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -186,6 +186,21 @@ Messages are processed in order: - Thread replies: `thread.messagePaginator`, owned by the `Thread` object (resolve via `client.threads`) — **independent** of the channel's message list. - **No cross-store invariant:** a reply is not required to exist in the channel's message list. Whether a reply also shows in the channel is the server's `show_in_channel` flag, applied when the message is ingested. +### Where `StateStore` comes from + +Import it from **`@stream-io/state-store`**, never from `stream-chat` (enforced by the +`state-store-single-source` and `react-compat` blocks in `eslint.config.mjs`). + +`stream-chat` re-exported the store until v10 extracted it into its own package, so the old +specifier still reads as correct — but it is now a plain `undefined` at runtime +(`TypeError: StateStore is not a constructor`), and nothing CI runs would catch it in a test file: +`yarn types` covers `tsconfig.lib.json` only. + +`stream-chat`, `@stream-io/i18n` and this package all depend on `@stream-io/state-store` and hand +each other store instances, so an app must end up with exactly one copy. A normal install dedupes +them; a linked `stream-chat` checkout does not, which is what `resolve.dedupe` in +`vitest.config.ts` covers for test runs. + ### React Version Compatibility SDK supports **React 17, 18, 19**. diff --git a/eslint.config.mjs b/eslint.config.mjs index 5cc650003..793cec55b 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -8,6 +8,27 @@ import sortDestructureKeysPlugin from 'eslint-plugin-sort-destructure-keys'; import reactPlugin from 'eslint-plugin-react'; import vitestPlugin from '@vitest/eslint-plugin'; +// `stream-chat` re-exported the state store until v10 extracted it into its own package, so the +// old specifier still reads as correct and typechecks nowhere CI looks (test files are outside +// `yarn types`). It resolves to `undefined` at runtime — "StateStore is not a constructor". +// `@stream-io/state-store` is the single source for this API; see CLAUDE.md. +const stateStoreFromStreamChat = { + name: 'stream-chat', + importNames: [ + 'StateStore', + 'MergedStateStore', + 'isPatch', + 'Patch', + 'ValueOrPatch', + 'Handler', + 'Unsubscribe', + 'RemovePreprocessor', + 'Preprocessor', + ], + message: + "`stream-chat` does not export the state store — it lives in `@stream-io/state-store`. Import it from there (a value import from 'stream-chat' resolves to undefined at runtime).", +}; + export default tseslint.config( { ignores: [ @@ -146,6 +167,7 @@ export default tseslint.config( message: 'React 18+/19-only API. Use useStableId from src/utils/useStableId, useSyncExternalStore from use-sync-external-store/shim. useEffectEvent and use() are not allowed: SDK supports React 17+.', }, + stateStoreFromStreamChat, ], }, ], @@ -171,6 +193,15 @@ export default tseslint.config( ], }, }, + { + // The `react-compat` block above ignores tests, but the state store specifier is wrong + // everywhere — and a test is where the wrong one actually reached CI. + name: 'state-store-single-source', + files: ['src/**/__tests__/**', 'src/mock-builders/**'], + rules: { + 'no-restricted-imports': ['error', { paths: [stateStoreFromStreamChat] }], + }, + }, { name: 'vitest', files: ['src/**/__tests__/**', 'src/mock-builders/**'], diff --git a/examples/vite/src/ChatLayout/WorkspaceUrlSync.tsx b/examples/vite/src/ChatLayout/WorkspaceUrlSync.tsx index 80a923d53..5a6cec00d 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, @@ -214,17 +238,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 69050f443..ed1a5326f 100644 --- a/src/components/MessageList/MessageList.tsx +++ b/src/components/MessageList/MessageList.tsx @@ -51,6 +51,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; @@ -234,6 +235,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; @@ -386,8 +390,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 9d9c2d664..534965aa7 100644 --- a/src/components/MessageList/VirtualizedMessageList.tsx +++ b/src/components/MessageList/VirtualizedMessageList.tsx @@ -74,6 +74,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'; @@ -491,18 +492,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/__tests__/useCanPaginateReplies.test.tsx b/src/components/MessageList/hooks/__tests__/useCanPaginateReplies.test.tsx new file mode 100644 index 000000000..0311434c5 --- /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-io/state-store'; +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 new file mode 100644 index 000000000..c1dc1e9d2 --- /dev/null +++ b/src/components/MessageList/hooks/useCanPaginateReplies.ts @@ -0,0 +1,53 @@ +import { useThreadContext } from '../../Threads'; +import { useStateStore } from '../../../store'; + +import type { LocalMessage, ThreadState } from 'stream-chat'; + +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 left to fetch by scrolling. + * + * Always `true` outside a thread — a channel list paginates regardless. + * + * 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, and `loadedCount` follows the paginator, so both re-evaluate on their own. + */ +export const useCanPaginateReplies = (): boolean => { + const thread = useThreadContext(); + const { replyCount } = useStateStore(thread?.state, threadSelector) ?? {}; + const { loadedCount } = useStateStore( + thread?.messagePaginator?.state, + paginatorSelector, + ) ?? { loadedCount: 0 }; + + if (!thread) return true; + if (loadedCount === 0) return false; + return (replyCount ?? 0) > loadedCount; +}; diff --git a/src/components/Thread/Thread.tsx b/src/components/Thread/Thread.tsx index f1760c8c2..92759fc04 100644 --- a/src/components/Thread/Thread.tsx +++ b/src/components/Thread/Thread.tsx @@ -71,6 +71,7 @@ export const Thread = (props: ThreadProps) => { const selector = (nextValue: ThreadState) => ({ isStateStale: nextValue.isStateStale, parentMessage: nextValue.parentMessage, + replyCount: nextValue.replyCount, }); const messagePaginatorSelector = ({ @@ -105,7 +106,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, @@ -135,13 +136,23 @@ 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 reloading at `replyCount` 0 + // can only 404 — `Thread.reload()` swallows that and returns without state. + // + // Deferred, not cancelled: only a successful reload clears `isStateStale`, so a thread that + // stays stale reloads via the effect below as soon as `replyCount` goes above 0 — the same + // moment the rest of the UI learns about replies missed while unwatched. + 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, @@ -149,10 +160,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 add47b7f9..43768509b 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 }; @@ -294,6 +299,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 }); diff --git a/vitest.config.ts b/vitest.config.ts index 6fdf24720..362820d44 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -10,6 +10,18 @@ export default defineConfig({ alias: { 'mock-builders': resolve(__dirname, 'src/mock-builders'), }, + // `stream-chat`, `@stream-io/i18n` and this package each depend on `@stream-io/state-store`, + // and they hand each other store instances. A plain install dedupes them, but a linked + // `stream-chat` checkout brings its own nested copy along — two `StateStore` classes, so + // `instanceof` and identity checks across the boundary stop holding. Resolve these from the + // project root always, so a test run exercises one copy of each the way an app does. + dedupe: [ + '@stream-io/state-store', + '@stream-io/i18n', + 'stream-chat', + 'react', + 'react-dom', + ], }, test: { globals: true,