From b74778a57c522cfa6e8e83ae4bd275db2a660b6b Mon Sep 17 00:00:00 2001 From: martincupela Date: Tue, 8 Sep 2026 15:39:23 +0200 Subject: [PATCH 1/7] feat: add network connection hooks --- .../__tests__/connectionStateHooks.test.tsx | 174 ++++++++++++++++++ .../Chat/hooks/useNetworkConnectionState.ts | 50 +++++ .../Chat/hooks/useWSConnectionState.ts | 32 ++++ src/components/Chat/index.ts | 2 + src/mock-builders/index.ts | 21 ++- 5 files changed, 277 insertions(+), 2 deletions(-) create mode 100644 src/components/Chat/hooks/__tests__/connectionStateHooks.test.tsx create mode 100644 src/components/Chat/hooks/useNetworkConnectionState.ts create mode 100644 src/components/Chat/hooks/useWSConnectionState.ts diff --git a/src/components/Chat/hooks/__tests__/connectionStateHooks.test.tsx b/src/components/Chat/hooks/__tests__/connectionStateHooks.test.tsx new file mode 100644 index 000000000..56552e94e --- /dev/null +++ b/src/components/Chat/hooks/__tests__/connectionStateHooks.test.tsx @@ -0,0 +1,174 @@ +import React from 'react'; +import { act, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { NetworkStatusListenerRegistrar, StreamChat } from 'stream-chat'; + +import { Chat } from '../../Chat'; +import { useNetworkConnectionState } from '../useNetworkConnectionState'; +import { useNetworkConnectionStateSelector } from '../useNetworkConnectionState'; +import { useWSConnectionState } from '../useWSConnectionState'; +import { getTestClientWithUser } from '../../../../mock-builders'; + +/** An integrator's registration function, of the shape a platform API would be wrapped in. */ +const platformListener = () => { + let report: ((isOnline: boolean) => void) | undefined; + const registrar: NetworkStatusListenerRegistrar = (onStatusChange) => { + report = onStatusChange; + return vi.fn(); + }; + return { + registrar, + report: (isOnline: boolean) => { + if (!report) throw new Error('the registrar was never installed'); + act(() => report?.(isOnline)); + }, + }; +}; + +const renderUnderChat = (client: StreamChat, ui: React.ReactNode) => + render({ui}); + +describe('connection state hooks', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('useNetworkConnectionState', () => { + it('starts online in jsdom, because jsdom is a browser', async () => { + // Worth stating explicitly: the SDK installs its built-in browser registrar whenever + // `window.addEventListener` exists and `navigator.onLine` is a boolean — which is true in + // jsdom. So a React test starts with a *known* status, not an unknown one, and the + // `undefined` case has to be arranged deliberately (see the next test). + const client = await getTestClientWithUser({ id: 'me' }); + const Consumer = () => { + const state = useNetworkConnectionState(); + return
{String(state?.isOnline)}
; + }; + + renderUnderChat(client, ); + + expect(screen.getByTestId('v')).toHaveTextContent('true'); + }); + + it('surfaces isOnline === undefined as-is where no registrar can be installed', async () => { + // The React Native shape: `navigator` has no boolean `onLine`, so no default registrar is + // installed and the status stays unknown. Stubbed before the client is constructed, because + // the registrar installs and reports during construction. + vi.stubGlobal('navigator', { userAgent: 'ReactNative' }); + const client = await getTestClientWithUser({ id: 'me' }); + const Consumer = () => { + const state = useNetworkConnectionState(); + return
{JSON.stringify(state?.isOnline ?? 'unknown')}
; + }; + + renderUnderChat(client, ); + + // Not coerced to false, and nothing crashed on the absent value. + expect(screen.getByTestId('v')).toHaveTextContent('"unknown"'); + vi.unstubAllGlobals(); + }); + + it('re-renders when the registrar reports a change', async () => { + const client = await getTestClientWithUser({ id: 'me' }); + const platform = platformListener(); + client.config.set({ + client: { networkConnection: { statusListenerRegistrar: platform.registrar } }, + }); + const Consumer = () => { + const state = useNetworkConnectionState(); + return
{String(state?.isOnline)}
; + }; + + renderUnderChat(client, ); + // Replacing the registrar does not reset the last known status — an edge is not a state — so + // this starts from what the browser registrar already reported. + expect(screen.getByTestId('v')).toHaveTextContent('true'); + + platform.report(false); + expect(screen.getByTestId('v')).toHaveTextContent('false'); + + platform.report(true); + expect(screen.getByTestId('v')).toHaveTextContent('true'); + }); + }); + + describe('useNetworkConnectionStateSelector', () => { + it('does not re-render when an unselected field changes', async () => { + const client = await getTestClientWithUser({ id: 'me' }); + const platform = platformListener(); + client.config.set({ + client: { networkConnection: { statusListenerRegistrar: platform.registrar } }, + }); + const renders = vi.fn(); + const Consumer = () => { + // `lastOnlineAt` and `lastOfflineAt` also change on every report; this selects neither. + const selected = useNetworkConnectionStateSelector(({ isOnline }) => ({ + isOnline, + })); + renders(); + return
{String(selected?.isOnline)}
; + }; + + renderUnderChat(client, ); + const initial = renders.mock.calls.length; + + // `false` is a real change from the browser registrar's initial `true`. + platform.report(false); + expect(renders.mock.calls.length).toBeGreaterThan(initial); + const afterChange = renders.mock.calls.length; + + // A repeat of the same status is ignored by the observer, so nothing re-renders. + platform.report(false); + expect(renders.mock.calls.length).toBe(afterChange); + }); + }); + + describe('the two hooks together', () => { + it('distinguishes network-up/socket-down from network-down/socket-up', async () => { + // If these two are not distinguishable, the feature has not delivered its point. + const client = await getTestClientWithUser({ id: 'me' }); + const platform = platformListener(); + client.config.set({ + client: { networkConnection: { statusListenerRegistrar: platform.registrar } }, + }); + const Consumer = () => { + // Aliased deliberately: both stores expose `isOnline`, so a blind destructure of both + // would shadow one with the other. + const { isOnline: networkOnline } = useNetworkConnectionState() ?? {}; + const { isOnline: socketOnline } = useWSConnectionState() ?? {}; + return ( +
{`network=${String(networkOnline)} socket=${String(socketOnline)}`}
+ ); + }; + + renderUnderChat(client, ); + // The mock client marks the socket up, so this is the interesting asymmetry: a device with no + // network while the socket still believes it is fine. + platform.report(false); + expect(screen.getByTestId('v')).toHaveTextContent('network=false socket=true'); + + // And the reverse: network fine, socket down. + platform.report(true); + act(() => { + client.wsConnection.state.partialNext({ isOnline: false }); + }); + expect(screen.getByTestId('v')).toHaveTextContent('network=true socket=false'); + }); + }); + + describe('useWSConnectionState', () => { + it('reports the socket and its connection id', async () => { + const client = await getTestClientWithUser({ id: 'me' }); + const Consumer = () => { + const state = useWSConnectionState(); + return ( +
{`${String(state?.isOnline)}/${String(state?.connectionId)}`}
+ ); + }; + + renderUnderChat(client, ); + + expect(screen.getByTestId('v')).toHaveTextContent('true/dummy_connection_id'); + }); + }); +}); diff --git a/src/components/Chat/hooks/useNetworkConnectionState.ts b/src/components/Chat/hooks/useNetworkConnectionState.ts new file mode 100644 index 000000000..8e4a50c08 --- /dev/null +++ b/src/components/Chat/hooks/useNetworkConnectionState.ts @@ -0,0 +1,50 @@ +import type { NetworkConnectionState } from 'stream-chat'; + +import { useChatContext } from '../../../context/ChatContext'; +import { useStateStore } from '../../../store'; + +const identity = (state: NetworkConnectionState) => state; + +/** + * The **device's** network status, as reported by the platform listener registered on + * `client.networkConnection`. + * + * Not the same fact as {@link useWSConnectionState}, and the difference is the point: a socket dies on + * a working network (a server close, an expired token, a health-check timeout), and a device drops + * while the socket has not noticed yet. Use this for "you're offline"; use the WebSocket hook for + * "reconnecting…". + * + * `isOnline` has **three** states. `undefined` means *unknown* — nobody has told the client, because + * no registrar is installed or one is installed and has not reported yet. So a guard must test + * `isOnline === false`; `!isOnline` is also true when the answer is unknown and would claim "offline" + * on any host without a registrar. (The WebSocket store's `isOnline` is always a boolean, so `!` is + * fine there.) + * + * Must run under `ChatProvider`, e.g. from a child of ``. + */ +export const useNetworkConnectionState = (): NetworkConnectionState | undefined => { + const { client } = useChatContext(); + return useStateStore(client?.networkConnection.state, identity); +}; + +/** + * {@link useNetworkConnectionState} narrowed to what a component actually reads, so it re-renders + * only when that changes. + * + * The selector must return a flat object or tuple — it is shallow-compared on its own keys. + * + * @example + * ```tsx + * const isOffline = useNetworkConnectionStateSelector( + * ({ isOnline }) => ({ isOffline: isOnline === false }), + * )?.isOffline; + * ``` + */ +export const useNetworkConnectionStateSelector = < + O extends Readonly | Readonly>, +>( + selector: (state: NetworkConnectionState) => O, +): O | undefined => { + const { client } = useChatContext(); + return useStateStore(client?.networkConnection.state, selector); +}; diff --git a/src/components/Chat/hooks/useWSConnectionState.ts b/src/components/Chat/hooks/useWSConnectionState.ts new file mode 100644 index 000000000..37c98ef35 --- /dev/null +++ b/src/components/Chat/hooks/useWSConnectionState.ts @@ -0,0 +1,32 @@ +import type { WSConnectionState } from 'stream-chat'; + +import { useChatContext } from '../../../context/ChatContext'; +import { useStateStore } from '../../../store'; + +const identity = (state: WSConnectionState) => state; + +/** + * This client's **WebSocket** status — whether the realtime connection is up, and the connection id + * the server keys channel watches by. + * + * Not the device's network: see {@link useNetworkConnectionState}. Use this for "reconnecting…", for + * disabling a composer, or for anything that needs the realtime connection specifically. + * + * Two things worth knowing about the underlying store: + * + * - It reports transitions the `connection.changed` event does not. That event is not dispatched by + * `client.closeConnection()` — the documented mobile backgrounding path — nor by two internal error + * paths, while this store is written on every transition. + * - It publishes a drop immediately, where the event waits five seconds to avoid strobing a + * "connection lost" banner on a brief flap. + * + * `connectionId` is assigned on a successful connect and **never cleared**, so a value there means + * "connected at some point", not "connected now" — read `isOnline` for that. Unlike the network + * store's, this `isOnline` is always a boolean. + * + * Must run under `ChatProvider`, e.g. from a child of ``. + */ +export const useWSConnectionState = (): WSConnectionState | undefined => { + const { client } = useChatContext(); + return useStateStore(client?.wsConnection.state, identity); +}; diff --git a/src/components/Chat/index.ts b/src/components/Chat/index.ts index 68e891507..e27bc6270 100644 --- a/src/components/Chat/index.ts +++ b/src/components/Chat/index.ts @@ -2,3 +2,5 @@ export * from './Chat'; export * from './hooks/useChat'; export * from './hooks/useReportLostConnectionSystemNotification'; export * from './hooks/useCreateChatClient'; +export * from './hooks/useNetworkConnectionState'; +export * from './hooks/useWSConnectionState'; diff --git a/src/mock-builders/index.ts b/src/mock-builders/index.ts index 0e1810d7f..ae5fd459b 100644 --- a/src/mock-builders/index.ts +++ b/src/mock-builders/index.ts @@ -8,12 +8,29 @@ const token = 'dummy_token'; const connectUser = (client: StreamChat, user: Partial) => new Promise((resolve) => { - client['connectionId'] = 'dumm_connection_id'; + // Mark the socket up, which is what "connected" means to the client: `channel.watch()` and + // `client.queryChannels()` wait for a live connection instead of degrading to `watch: false`. + // + // Written through the public store rather than the socket's internal `_setStatus`, which + // `no-underscore-dangle` rightly rejects — and a fixture standing in for a connection it never + // opens is exactly the case for setting the state directly. `lastOnlineAt` is stamped too, since + // online-without-a-timestamp is a state the real socket never produces. + // + // This replaces `client['connectionId'] = '…'`, which was dead — the client has no such field. + // The connection id lives on the socket, so that assignment never satisfied the old + // `_hasConnectionID()` guard, and every mocked `watch()` quietly took the downgrade path. + client.wsConnection.state.partialNext({ + connectionId: 'dummy_connection_id', + isOnline: true, + lastOnlineAt: new Date(), + }); client.user = { ...user, mutes: [] } as UserResponse; client['_user'] = { ...user } as UserResponse; // `userID` is a getter in v10 (derives from `client.user?.id`), so it can't be assigned; // setting `client.user` above is what populates it. - client['userToken'] = token; + // `userToken` was never a field on `StreamChat` — only `userTokenOrProvider`, a parameter — so + // this assignment wrote a property nothing reads. The `tokenManager` mock below is what actually + // supplies the token. client.wsPromise = Promise.resolve() as StreamChat['wsPromise']; resolve(); }); From f0cff3c7bb04d7d54cd181446b97f7c854cc79cc Mon Sep 17 00:00:00 2001 From: martincupela Date: Tue, 8 Sep 2026 16:22:42 +0200 Subject: [PATCH 2/7] refactor: remove unused ref online in Channel --- src/components/Channel/Channel.tsx | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/components/Channel/Channel.tsx b/src/components/Channel/Channel.tsx index 31c200515..471685d25 100644 --- a/src/components/Channel/Channel.tsx +++ b/src/components/Channel/Channel.tsx @@ -126,7 +126,6 @@ const ChannelInner = ( const jumpToMessageFromSearch = useSearchFocusedMessage(); const originalTitle = useRef(''); - const online = useRef(true); const clearSearchFocusedMessageTimeoutId = useRef | null>( null, @@ -146,10 +145,6 @@ const ChannelInner = ( if (event.type === 'user.watching.start' || event.type === 'user.watching.stop') return; - if (event.type === 'connection.changed' && typeof event.online === 'boolean') { - online.current = event.online; - } - if (event.type === 'connection.recovered') { // Refresh the loaded message window ourselves. The client's reconnect hydration deliberately // skips re-seeding the message list of an `active` channel (we mark this one active while @@ -160,8 +155,9 @@ const ChannelInner = ( // surfaces them). This is deliberately the SDK's opinion about how the default component // behaves, not client-level policy. // - // `recoverState` dispatches this only after re-querying the active channels, so the rest of the - // channel state is already fresh by now. + // `ConnectionRecoveryManager` dispatches this only after re-querying the active channels, so + // the rest of the channel state is already fresh by now. (It replaced `client.recoverState()`, + // which is what this comment used to name.) if (channel.pendingDisposal) return; try { await channel.reload(); From db7681048805fc831f21bf7641b0e627266dddf8 Mon Sep 17 00:00:00 2001 From: martincupela Date: Wed, 9 Sep 2026 13:29:54 +0200 Subject: [PATCH 3/7] fix: keep the lost-connection notification when the socket drops during i18n init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The persistent "Waiting for network…" notification was removed moments after being published, whenever the socket dropped before `Streami18n.init()` resolved — an offline app launch, a captive portal, an expired token. That is precisely when the banner is wanted, and there was none. Dismissal was part of the subscription effect's cleanup, so any change to that effect's dependencies destroyed the notification, and `t` changes identity when init completes (the constructed placeholder gives way to i18next's real one). Re-subscribing does not republish, so the banner was gone for good. Dismissal is now scoped to the mount, which is what it was always for. `t` stays in the subscription's dependencies, where the lint rule wants it; re-subscribing on a change is idempotent and leaves the notification alone. The existing test passed only because `waitFor` polls and caught the transient window between the notification being published and being removed. The new test drops the socket without awaiting anything first, so it lands inside the init window, and fails if dismissal is moved back into the subscription's cleanup. --- src/components/Chat/__tests__/Chat.test.tsx | 28 +++++++++++++++++ ...eReportLostConnectionSystemNotification.ts | 31 ++++++++++++------- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/components/Chat/__tests__/Chat.test.tsx b/src/components/Chat/__tests__/Chat.test.tsx index 3c5d37644..d18e5d2e7 100644 --- a/src/components/Chat/__tests__/Chat.test.tsx +++ b/src/components/Chat/__tests__/Chat.test.tsx @@ -376,6 +376,34 @@ describe('Chat', () => { }); describe('connection notifications', () => { + it('keeps the notification when the socket drops before i18n has initialized', async () => { + // The regression. `Streami18n.init()` is asynchronous and `t` changes identity when it + // resolves. With `t` in the effect's dependencies, a drop during that window published the + // notification and then had it dismissed by the effect's own cleanup — leaving no banner + // exactly when one is most wanted: an offline app launch, a captive portal, an expired token. + const client = await getTestClientWithUser(); + render( + +
+ , + ); + const chatNotifications = () => + client.notifications.notifications.filter( + (notification) => notification.origin.emitter === 'Chat', + ); + + // Deliberately not awaiting anything first — the drop lands inside the init window. + act(() => dispatchConnectionChangedEvent(client, false)); + expect(chatNotifications()).toHaveLength(1); + + // Long enough for `init()` to resolve and `t` to be replaced. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)); + }); + + expect(chatNotifications()).toHaveLength(1); + }); + it('publishes and removes system connection-lost notification on connection changes', async () => { const client = getTestClient(); let connectionLostNotification; diff --git a/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts b/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts index e916f29da..797635790 100644 --- a/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts +++ b/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef } from 'react'; +import { useCallback, useEffect, useRef } from 'react'; import type { EventPayload } from 'stream-chat'; import { useChatContext } from '../../../context/ChatContext'; @@ -15,15 +15,25 @@ export const useReportLostConnectionSystemNotification = () => { const { addSystemNotification, removeNotification } = useNotificationApi(); const connectionLostNotificationIdRef = useRef(null); + const dismissConnectionLostNotification = useCallback(() => { + if (!connectionLostNotificationIdRef.current) return; + removeNotification(connectionLostNotificationIdRef.current); + connectionLostNotificationIdRef.current = null; + }, [removeNotification]); + + /** + * Dismissal is scoped to the mount, not to the subscription below. + * + * The subscription's dependencies change for reasons that have nothing to do with the connection — + * `t` is replaced when `Streami18n.init()` resolves, asynchronously. When dismissal was part of + * that effect's cleanup, a socket dropping before init finished had its notification published and + * then immediately removed, leaving no banner on an offline app launch or behind a captive portal. + */ + useEffect(() => dismissConnectionLostNotification, [dismissConnectionLostNotification]); + useEffect(() => { if (!t || !client) return; - const dismissConnectionLostNotification = () => { - if (!connectionLostNotificationIdRef.current) return; - removeNotification(connectionLostNotificationIdRef.current); - connectionLostNotificationIdRef.current = null; - }; - const handleConnectionChanged = ({ online }: EventPayload<'connection.changed'>) => { if (!online) { if (connectionLostNotificationIdRef.current) return; @@ -46,9 +56,6 @@ export const useReportLostConnectionSystemNotification = () => { const subscription = client.on('connection.changed', handleConnectionChanged); - return () => { - subscription.unsubscribe(); - dismissConnectionLostNotification(); - }; - }, [addSystemNotification, client, removeNotification, t]); + return subscription.unsubscribe; + }, [addSystemNotification, client, dismissConnectionLostNotification, t]); }; From ee278c62a19b3af099b0c4f1260fab164327bb48 Mon Sep 17 00:00:00 2001 From: martincupela Date: Wed, 9 Sep 2026 13:31:33 +0200 Subject: [PATCH 4/7] fix: narrow connection event handlers on the new connection field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `connection.changed` and `connection.recovered` carry `connection: 'network' | 'ws'` in stream-chat v10, and both variants reach every existing handler. A handler that ignores the field therefore reacts to the device's network as well as to our WebSocket, which would report a lost network every time the socket drops on a working one. Every subscription keeps its event name and its `online` field; each now narrows to `'ws'`, preserving today's behaviour exactly: - `useReportLostConnectionSystemNotification` publishes its banner for the socket only. The notification type says `network`, but the fact behind it has always been the socket. - `Channel` reloads its loaded message window on the socket's recovery only. The compiler cannot help here — both variants have the same payload shape, so an un-guarded handler keeps compiling and keeps behaving as before. Two tests fail if either guard is removed. `Channel` also drops its `connection.changed` subscription entirely. The branch that read it went with the dead `online` ref, and `handleEvent` has no catch-all, so the event reached the component and did nothing. The mock builders take an optional `ConnectionType`, defaulting to `'ws'` so existing call sites keep the meaning they had when this event could only be about the socket. --- src/components/Channel/Channel.tsx | 7 +++-- .../Channel/__tests__/Channel.test.tsx | 18 ++++++++++++ src/components/Chat/__tests__/Chat.test.tsx | 28 ++++++++++++++++++- ...eReportLostConnectionSystemNotification.ts | 11 +++++++- src/mock-builders/event/connectionChanged.ts | 16 +++++++++-- .../event/connectionRecovered.ts | 11 ++++++-- 6 files changed, 83 insertions(+), 8 deletions(-) diff --git a/src/components/Channel/Channel.tsx b/src/components/Channel/Channel.tsx index 471685d25..6956ee2c3 100644 --- a/src/components/Channel/Channel.tsx +++ b/src/components/Channel/Channel.tsx @@ -146,6 +146,11 @@ const ChannelInner = ( return; if (event.type === 'connection.recovered') { + // Only the socket's recovery reloads the window. Nothing dispatches a `'network'` recovery + // today, but the guard means that if something ever does, requerying every open channel off a + // network edge is a deliberate decision rather than a silent behavior change. + if (event.connection !== 'ws') return; + // Refresh the loaded message window ourselves. The client's reconnect hydration deliberately // skips re-seeding the message list of an `active` channel (we mark this one active while // mounted) because its 25-message page would perturb a larger scrolled-back window — it hands @@ -302,7 +307,6 @@ const ChannelInner = ( } // The more complex sync logic is done in Chat - client.on('connection.changed', handleEvent); client.on('connection.recovered', handleEvent); client.on('user.updated', handleEvent); client.on('user.deleted', handleEvent); @@ -314,7 +318,6 @@ const ChannelInner = ( isMounted = false; if (errored || !done) return; channel?.off(handleEvent); - client.off('connection.changed', handleEvent); client.off('connection.recovered', handleEvent); client.off('user.deleted', handleEvent); }; diff --git a/src/components/Channel/__tests__/Channel.test.tsx b/src/components/Channel/__tests__/Channel.test.tsx index 0ab48ff6c..480bd2e93 100644 --- a/src/components/Channel/__tests__/Channel.test.tsx +++ b/src/components/Channel/__tests__/Channel.test.tsx @@ -422,6 +422,24 @@ describe('Channel', () => { await waitFor(() => expect(reloadSpy).toHaveBeenCalledTimes(1)); }); + it('does not reload the channel on a recovery reported for the network', async () => { + // Nothing dispatches a `'network'` recovery today. The guard is what makes requerying every + // open channel off a network edge a deliberate decision if something ever does, rather than a + // silent behaviour change — and the compiler cannot catch its absence, because both variants + // of the event have the same shape. + const { channel, chatClient } = await setup(); + await renderComponent({ channel, chatClient }); + + const reloadSpy = vi.spyOn(channel, 'reload').mockResolvedValue(undefined); + + await act(async () => { + dispatchConnectionRecoveredEvent(chatClient, 'network'); + await Promise.resolve(); + }); + + expect(reloadSpy).not.toHaveBeenCalled(); + }); + it('does not reload a channel that is pending disposal', async () => { const { channel, chatClient } = await setup(); await renderComponent({ channel, chatClient }); diff --git a/src/components/Chat/__tests__/Chat.test.tsx b/src/components/Chat/__tests__/Chat.test.tsx index d18e5d2e7..4c1fd2bd9 100644 --- a/src/components/Chat/__tests__/Chat.test.tsx +++ b/src/components/Chat/__tests__/Chat.test.tsx @@ -393,7 +393,7 @@ describe('Chat', () => { ); // Deliberately not awaiting anything first — the drop lands inside the init window. - act(() => dispatchConnectionChangedEvent(client, false)); + act(() => dispatchConnectionChangedEvent(client, false, 'ws')); expect(chatNotifications()).toHaveLength(1); // Long enough for `init()` to resolve and `t` to be replaced. @@ -404,6 +404,32 @@ describe('Chat', () => { expect(chatNotifications()).toHaveLength(1); }); + it('publishes nothing when the DEVICE network drops, only when the socket does', async () => { + // This notification reports the WebSocket, despite `network` in its type name, and + // `connection.changed` now arrives for both connections — so without a `connection` guard it + // would fire for a device that lost its network on a perfectly healthy socket. The compiler + // cannot catch a missing guard: both variants of the event have the same shape. + const client = await getTestClientWithUser(); + render( + +
+ , + ); + const chatNotifications = () => + client.notifications.notifications.filter( + (notification) => notification.origin.emitter === 'Chat', + ); + + act(() => dispatchConnectionChangedEvent(client, false, 'network')); + + expect(chatNotifications()).toHaveLength(0); + + // The socket variant does publish, which is what makes the assertion above meaningful. + act(() => dispatchConnectionChangedEvent(client, false, 'ws')); + + expect(chatNotifications()).toHaveLength(1); + }); + it('publishes and removes system connection-lost notification on connection changes', async () => { const client = getTestClient(); let connectionLostNotification; diff --git a/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts b/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts index 797635790..dfc786c22 100644 --- a/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts +++ b/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts @@ -34,7 +34,16 @@ export const useReportLostConnectionSystemNotification = () => { useEffect(() => { if (!t || !client) return; - const handleConnectionChanged = ({ online }: EventPayload<'connection.changed'>) => { + const handleConnectionChanged = ({ + connection, + online, + }: EventPayload<'connection.changed'>) => { + // Narrowed to the socket, which is what this hook has always reported — the notification type + // says `network`, but the fact behind it is the WebSocket. Now that the event also arrives for + // the device's network, the guard is what keeps that unchanged rather than silently doubling. + // Whether a lost *network* deserves its own notification is a separate question. + if (connection !== 'ws') return; + if (!online) { if (connectionLostNotificationIdRef.current) return; diff --git a/src/mock-builders/event/connectionChanged.ts b/src/mock-builders/event/connectionChanged.ts index 7bc01e698..4c51b3f2e 100644 --- a/src/mock-builders/event/connectionChanged.ts +++ b/src/mock-builders/event/connectionChanged.ts @@ -1,9 +1,21 @@ import { fromPartial } from '@total-typescript/shoehorn'; -import type { Event, StreamChat } from 'stream-chat'; +import type { ConnectionType, Event, StreamChat } from 'stream-chat'; -export default (client: StreamChat, online: boolean) => { +/** + * Dispatches `connection.changed`. + * + * `connection` defaults to `'ws'` so existing call sites keep the meaning they had before the event + * gained a discriminator — every one of them was written when this event could only be about the + * WebSocket. Pass `'network'` to simulate the device's network instead. + */ +export default ( + client: StreamChat, + online: boolean, + connection: ConnectionType = 'ws', +) => { client.dispatchEvent( fromPartial({ + connection, online, type: 'connection.changed', }), diff --git a/src/mock-builders/event/connectionRecovered.ts b/src/mock-builders/event/connectionRecovered.ts index a311ff7b6..75359cfb9 100644 --- a/src/mock-builders/event/connectionRecovered.ts +++ b/src/mock-builders/event/connectionRecovered.ts @@ -1,9 +1,16 @@ import { fromPartial } from '@total-typescript/shoehorn'; -import type { Event, StreamChat } from 'stream-chat'; +import type { ConnectionType, Event, StreamChat } from 'stream-chat'; -export default (client: StreamChat) => { +/** + * Dispatches `connection.recovered`. + * + * `connection` defaults to `'ws'`, which is also the only value the client actually dispatches today — + * recovery is about the socket being back and its watches re-established. + */ +export default (client: StreamChat, connection: ConnectionType = 'ws') => { client.dispatchEvent( fromPartial({ + connection, type: 'connection.recovered', }), ); From 4d2137ec93ad807316573f71d30ae58bb8bf8e65 Mon Sep 17 00:00:00 2001 From: martincupela Date: Thu, 10 Sep 2026 10:46:10 +0200 Subject: [PATCH 5/7] fix: reload an open channel once per reconnect, not twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ConnectionRecoveryManager` reloads every active channel and then dispatches `connection.recovered`. `Channel` handled that event by calling `channel.reload()` again, and since it marks its channel active while mounted, every open channel was reloaded twice per reconnect — two full `watch()` requests each. `Channel.reload()`'s `_reloading` flag is a re-entrancy guard and has already reset by the time the event is dispatched, so it did not collapse the pair. Measured at two reloads per reconnect before this change and one after. The handler goes, and with it the `connection.recovered` subscription: nothing else in `handleEvent` reacted to that event. The reconciliation React's reload existed for — a hard delete that happened offline arrives via no event, so only a re-query surfaces it — still happens, because the client reloads active channels itself. The old test dispatched `connection.recovered` directly, which is exactly why it never saw the duplicate; the replacement drives a whole reconnect from the socket coming back. `keeps rendering when the reload fails` moves with the behaviour it covered: the client reloads with `Promise.allSettled`, so a socket flapping mid-reload cannot throw into the component at all. Note the `stream-chat` pin is deliberately left at 10.0.0-rc.7. It has to move to the release that carries the connection work — rc.9 exports none of `ConnectionType`, `NetworkConnectionState` or `client.networkConnection`, which this branch already imports — so bumping it is a release-time step, not part of this change. --- src/components/Channel/Channel.tsx | 32 ------------- .../Channel/__tests__/Channel.test.tsx | 48 +++++++++++-------- 2 files changed, 29 insertions(+), 51 deletions(-) diff --git a/src/components/Channel/Channel.tsx b/src/components/Channel/Channel.tsx index d1c11e635..724705a6a 100644 --- a/src/components/Channel/Channel.tsx +++ b/src/components/Channel/Channel.tsx @@ -135,7 +135,6 @@ const ChannelInner = ( !channel.initialized && initializeOnMount, ); - // todo: can we remove this big event handler and keep only relevant UI-only logic (e.g. 'connection.recovered')? const handleEvent = async (event: Event) => { // ignore the event if it is not targeted at the current channel. // Event targeted at this channel or globally targeted event should lead to state refresh @@ -145,35 +144,6 @@ const ChannelInner = ( if (event.type === 'user.watching.start' || event.type === 'user.watching.stop') return; - if (event.type === 'connection.recovered') { - // Only the socket's recovery reloads the window. Nothing dispatches a `'network'` recovery - // today, but the guard means that if something ever does, requerying every open channel off a - // network edge is a deliberate decision rather than a silent behavior change. - if (event.connection !== 'ws') return; - - // Refresh the loaded message window ourselves. The client's reconnect hydration deliberately - // skips re-seeding the message list of an `active` channel (we mark this one active while - // mounted) because its 25-message page would perturb a larger scrolled-back window — it hands - // that job to `channel.reload()`, which re-watches sized to the loaded window instead. Nothing - // calls it for us, so without this the list stays stale after a reconnect and hard deletes that - // happened while offline are never reconciled (they arrive via no event; only a re-query - // surfaces them). This is deliberately the SDK's opinion about how the default component - // behaves, not client-level policy. - // - // `ConnectionRecoveryManager` dispatches this only after re-querying the active channels, so - // the rest of the channel state is already fresh by now. (It replaced `client.recoverState()`, - // which is what this comment used to name.) - if (channel.pendingDisposal) return; - try { - await channel.reload(); - } catch (error) { - // The socket can flap straight back down mid-reload. Keep the previously loaded window - // rather than tearing the view down — the next recovery re-runs this. - console.warn('Failed to reload the channel after connection recovery', error); - } - return; - } - if (event.type === 'message.new') { const mainChannelUpdated = !event.message?.parent_id || event.message?.show_in_channel; @@ -307,7 +277,6 @@ const ChannelInner = ( } // The more complex sync logic is done in Chat - client.on('connection.recovered', handleEvent); client.on('user.updated', handleEvent); client.on('user.deleted', handleEvent); client.on('user.messages.deleted', handleEvent); @@ -318,7 +287,6 @@ const ChannelInner = ( isMounted = false; if (errored || !done) return; channel?.off(handleEvent); - client.off('connection.recovered', handleEvent); client.off('user.deleted', handleEvent); }; // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/src/components/Channel/__tests__/Channel.test.tsx b/src/components/Channel/__tests__/Channel.test.tsx index 04f56de43..70dfed8c1 100644 --- a/src/components/Channel/__tests__/Channel.test.tsx +++ b/src/components/Channel/__tests__/Channel.test.tsx @@ -406,35 +406,42 @@ describe('Channel', () => { }); describe('connection recovery', () => { - // The client's reconnect hydration skips re-seeding the message list of an `active` channel - // (Channel marks it active while mounted) and delegates that window to `channel.reload()`. - // Nothing else calls it, so these pin the SDK component as the thing that does. - it('reloads the channel when the connection is recovered', async () => { + // `ConnectionRecoveryManager` reloads every active channel and *then* dispatches + // `connection.recovered`. `Channel` used to handle that event by reloading again, and since it + // marks its channel active while mounted, every open channel was reloaded twice per reconnect — + // two full `watch()` requests. `Channel.reload()`'s `_reloading` flag is a re-entrancy guard and + // has already reset by the time the event is dispatched, so it did not collapse the pair. + it('reloads an open channel exactly once per reconnect', async () => { const { channel, chatClient } = await setup(); await renderComponent({ channel, chatClient }); + await waitFor(() => expect(channel.active).toBe(true)); const reloadSpy = vi.spyOn(channel, 'reload').mockResolvedValue(undefined); + chatClient.connectionRecovery.registerSubscriptions(); + // A whole reconnect, driven from the socket coming back rather than by dispatching the + // recovery event directly — that is what exercises both would-be reloaders. await act(async () => { - dispatchConnectionRecoveredEvent(chatClient); - await Promise.resolve(); + dispatchConnectionChangedEvent(chatClient, true, 'ws'); + await new Promise((resolve) => setTimeout(resolve, 50)); }); - await waitFor(() => expect(reloadSpy).toHaveBeenCalledTimes(1)); + expect(reloadSpy).toHaveBeenCalledTimes(1); }); - it('does not reload the channel on a recovery reported for the network', async () => { - // Nothing dispatches a `'network'` recovery today. The guard is what makes requerying every - // open channel off a network edge a deliberate decision if something ever does, rather than a - // silent behaviour change — and the compiler cannot catch its absence, because both variants - // of the event have the same shape. + it('leaves the reload to the client, so it happens even without this component', async () => { + // The reconciliation React's own reload existed for — a hard delete that happened offline + // arrives via no event, so only a re-query surfaces it — still happens, because the client + // reloads active channels itself. This pins that it is the client doing it. const { channel, chatClient } = await setup(); await renderComponent({ channel, chatClient }); + await waitFor(() => expect(channel.active).toBe(true)); const reloadSpy = vi.spyOn(channel, 'reload').mockResolvedValue(undefined); + // No `connection.recovered` handler in `Channel` any more, so this alone must do nothing. await act(async () => { - dispatchConnectionRecoveredEvent(chatClient, 'network'); + dispatchConnectionRecoveredEvent(chatClient); await Promise.resolve(); }); @@ -457,23 +464,26 @@ describe('Channel', () => { expect(reloadSpy).not.toHaveBeenCalled(); }); - it('keeps rendering when the reload fails', async () => { + it('keeps rendering when the reload fails during a reconnect', async () => { + // The guarantee the old React-side error handling provided, now held somewhere better: the + // client reloads active channels with `Promise.allSettled`, so a socket that flaps back down + // mid-reload cannot throw into this component at all. const { channel, chatClient } = await setup(); const { container } = await renderComponent({ channel, chatClient }); + await waitFor(() => expect(channel.active).toBe(true)); - const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); const reloadSpy = vi .spyOn(channel, 'reload') .mockRejectedValue(new Error('socket flapped')); + chatClient.connectionRecovery.registerSubscriptions(); await act(async () => { - dispatchConnectionRecoveredEvent(chatClient); - await Promise.resolve(); + dispatchConnectionChangedEvent(chatClient, true, 'ws'); + await new Promise((resolve) => setTimeout(resolve, 50)); }); - await waitFor(() => expect(reloadSpy).toHaveBeenCalledTimes(1)); + expect(reloadSpy).toHaveBeenCalledTimes(1); expect(container.querySelector('.str-chat__channel')).toBeInTheDocument(); - warnSpy.mockRestore(); }); }); From 456b0b6d0228d36f8090ea3898ea8c18bd135bac Mon Sep 17 00:00:00 2001 From: martincupela Date: Thu, 10 Sep 2026 10:46:17 +0200 Subject: [PATCH 6/7] fix: stop the offline banner blaming the network for a dead socket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The persistent banner read "Waiting for network…" whenever `connection.changed` reported `online: false`. That event is the WebSocket, so the message told users their network was down when the server had closed the socket, the token had expired, or a health check had timed out on working Wi-Fi. It now reads both facts and picks the wording: - the device reports no network → "Waiting for network…" - the network is up, or unknown, and the socket is down → "Reconnecting…" One banner throughout, replaced rather than stacked when the reason changes. Choosing that grouping is a copy decision rather than a fact about connectivity, which is why the client publishes no combined status and why the decision is made here, in the component that renders the copy. The existing translation key keeps the network case, where its wording was always true, so its translations stay valid and only one new string was needed. It also reads the current state at mount. It reacted only to transitions before, so a client that was already offline showed nothing until something changed. Two things kept deliberately: - One notification type for both messages. Consumers filter banners on `system:network:connection:lost` — the SDK's own cookbook recipe does — so splitting it would silently stop those filters seeing the socket case. The `network` in the name is historical; the message is what was wrong. - The socket half stays on `connection.changed` rather than `client.wsConnection.state`. The event is held `WS_OFFLINE_ANNOUNCE_DELAY_MS` (5s) on the way down and dropped entirely if the socket returns inside that window, which is what stops a brief flap strobing the banner; the store publishes the raw edge. The socket's last announced value therefore lives in a ref seeded from the store only once — re-reading the store on every effect re-run discarded what the event had said and dismissed the banner. `ConnectionType` is imported from the client rather than redeclared, so a connection type added there breaks `yarn build` here until someone decides what the banner should say about it. Both signals are subscribed imperatively rather than through `useNetworkConnectionState` / `useWSConnectionState`: `Chat` calls this hook, and re-rendering the whole tree on every network flap is what those hooks exist to let consumers avoid. --- src/components/Chat/__tests__/Chat.test.tsx | 123 +++++++++++---- ...eReportLostConnectionSystemNotification.ts | 144 +++++++++++++----- src/i18n/__tests__/catalog.fixture.json | 1 + src/i18n/keys.ts | 1 + 4 files changed, 199 insertions(+), 70 deletions(-) diff --git a/src/components/Chat/__tests__/Chat.test.tsx b/src/components/Chat/__tests__/Chat.test.tsx index 4c1fd2bd9..d35b6285e 100644 --- a/src/components/Chat/__tests__/Chat.test.tsx +++ b/src/components/Chat/__tests__/Chat.test.tsx @@ -404,35 +404,75 @@ describe('Chat', () => { expect(chatNotifications()).toHaveLength(1); }); - it('publishes nothing when the DEVICE network drops, only when the socket does', async () => { - // This notification reports the WebSocket, despite `network` in its type name, and - // `connection.changed` now arrives for both connections — so without a `connection` guard it - // would fire for a device that lost its network on a perfectly healthy socket. The compiler - // cannot catch a missing guard: both variants of the event have the same shape. + /** + * The device losing its network and the socket dying are different facts, so they get different + * copy. Publishing "Waiting for network…" off the socket alone — which is what this did — told + * users their network was down when the server had closed the socket, the token had expired or a + * health check had timed out on working Wi-Fi. + */ + const chatNotificationsOf = (client: StreamChat) => + client.notifications.notifications.filter( + (notification) => notification.origin.emitter === 'Chat', + ); + + it('says reconnecting, not offline, when the socket dies on a working network', async () => { const client = await getTestClientWithUser(); render(
, ); - const chatNotifications = () => - client.notifications.notifications.filter( - (notification) => notification.origin.emitter === 'Chat', - ); + // jsdom is a browser, so the built-in registrar has already reported the network as up. + expect(client.networkConnection.isOnline).toBe(true); - act(() => dispatchConnectionChangedEvent(client, false, 'network')); + act(() => dispatchConnectionChangedEvent(client, false, 'ws')); - expect(chatNotifications()).toHaveLength(0); + await waitFor(() => expect(chatNotificationsOf(client)).toHaveLength(1)); + expect(chatNotificationsOf(client)[0].message).toBe('Reconnecting…'); + expect(chatNotificationsOf(client)[0].tags).toEqual(['system']); + }); + + it('says the network is down when the device reports no network', async () => { + const client = await getTestClientWithUser(); + render( + +
+ , + ); + + act(() => client.networkConnection.setStatus(false)); + + await waitFor(() => expect(chatNotificationsOf(client)).toHaveLength(1)); + expect(chatNotificationsOf(client)[0].message).toBe('Waiting for network…'); + }); + + it('swaps to the network message when the network drops while reconnecting', async () => { + const client = await getTestClientWithUser(); + render( + +
+ , + ); - // The socket variant does publish, which is what makes the assertion above meaningful. act(() => dispatchConnectionChangedEvent(client, false, 'ws')); + await waitFor(() => + expect(chatNotificationsOf(client)[0].message).toBe('Reconnecting…'), + ); - expect(chatNotifications()).toHaveLength(1); + act(() => client.networkConnection.setStatus(false)); + + // One banner throughout, with the more specific message replacing the general one. + await waitFor(() => + expect(chatNotificationsOf(client)[0].message).toBe('Waiting for network…'), + ); + expect(chatNotificationsOf(client)).toHaveLength(1); }); - it('publishes and removes system connection-lost notification on connection changes', async () => { - const client = getTestClient(); - let connectionLostNotification; + it('publishes immediately when the client is already offline at mount', async () => { + // It used to react only to transitions, so a client that was already offline showed nothing + // until something changed. + const client = await getTestClientWithUser(); + client.networkConnection.setStatus(false); render( @@ -440,27 +480,44 @@ describe('Chat', () => { , ); - expect(client.notifications.notifications).toHaveLength(0); + await waitFor(() => expect(chatNotificationsOf(client)).toHaveLength(1)); + expect(chatNotificationsOf(client)[0].message).toBe('Waiting for network…'); + }); - act(() => dispatchConnectionChangedEvent(client, false)); - await waitFor(() => { - connectionLostNotification = client.notifications.notifications.find( - (notification) => notification.origin.emitter === 'Chat', - ); - expect(connectionLostNotification).toBeDefined(); - }); + it('clears on recovery', async () => { + const client = await getTestClientWithUser(); + render( + +
+ , + ); - expect(connectionLostNotification.message).toBe('Waiting for network…'); - expect(connectionLostNotification.tags).toEqual(['system']); + act(() => dispatchConnectionChangedEvent(client, false, 'ws')); + await waitFor(() => expect(chatNotificationsOf(client)).toHaveLength(1)); - act(() => dispatchConnectionChangedEvent(client, true)); - await waitFor(() => { - expect( - client.notifications.notifications.find( - (notification) => notification.origin.emitter === 'Chat', - ), - ).toBeUndefined(); + act(() => dispatchConnectionChangedEvent(client, true, 'ws')); + + await waitFor(() => expect(chatNotificationsOf(client)).toHaveLength(0)); + }); + + it('takes the socket from the debounced event, not from the raw store', async () => { + // The going-offline delay lives on `connection.changed`: it is held for five seconds and + // dropped entirely if the socket returns inside that window, which is what stops a brief flap + // strobing the banner. `client.wsConnection.state` publishes the raw edge instead, so reading + // the socket from there would lose the anti-flicker. + const client = await getTestClientWithUser(); + render( + +
+ , + ); + + act(() => client.wsConnection.state.partialNext({ isOnline: false })); + await act(async () => { + await Promise.resolve(); }); + + expect(chatNotificationsOf(client)).toHaveLength(0); }); it('uses NotificationAnnouncer from ComponentContext', async () => { diff --git a/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts b/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts index dfc786c22..447724cb3 100644 --- a/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts +++ b/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts @@ -1,70 +1,140 @@ import { useCallback, useEffect, useRef } from 'react'; -import type { EventPayload } from 'stream-chat'; +import type { ConnectionType } from 'stream-chat'; import { useChatContext } from '../../../context/ChatContext'; import { useTranslationContext } from '../../../context/TranslationContext'; import { useNotificationApi } from '../../Notifications/hooks/useNotificationApi'; /** - * Publishes a persistent system notification while the client is offline and removes it when - * back online. Must run under `ChatProvider` and `TranslationProvider` (e.g. from a child of ``). + * Publishes a persistent system notification while this client cannot reach Stream, and removes it + * when it can again. Must run under `ChatProvider` and `TranslationProvider` (e.g. from a child of + * ``). + * + * **Two facts, two messages.** The device losing its network and this client's WebSocket dying are + * different things, and they disagree in both directions — a socket dies on working Wi-Fi when the + * server closes it, the token expires or a health check times out. This hook used to publish + * "Waiting for network…" off the socket alone, so it told users their network was down when it was + * fine. It now reads both and picks the wording: + * + * - the device reports no network → the network message + * - the network is up (or unknown) and the socket is down → the reconnecting message + * + * Choosing that grouping is a copy decision rather than a fact about connectivity, which is why the + * SDK publishes no combined status and why the decision is made here, in the component that renders + * the copy. + * + * Both signals are subscribed **imperatively** rather than through the `useNetworkConnectionState` / + * `useWSConnectionState` hooks: `` calls this, and re-rendering the whole tree on every network + * flap is exactly what those hooks exist to let consumers avoid. */ export const useReportLostConnectionSystemNotification = () => { const { t } = useTranslationContext(); const { client } = useChatContext(); const { addSystemNotification, removeNotification } = useNotificationApi(); - const connectionLostNotificationIdRef = useRef(null); + const notificationIdRef = useRef(null); + /** Outside the effect, so re-establishing the subscriptions does not republish what is showing. */ + const reasonRef = useRef(null); + /** + * The socket's status as last *announced*, which is not the same as `client.wsConnection.state`. + * + * It lives outside the effect because the effect re-runs for unrelated reasons (`t` is replaced + * when `Streami18n.init()` resolves) and the store cannot be used to re-seed it: the store carries + * the raw edge while the event carries the debounced one, so re-reading the store on every setup + * would discard what the event last said. Seeded from the store once, because that is the only + * honest answer before any event has arrived. + */ + const socketOnlineRef = useRef(null); const dismissConnectionLostNotification = useCallback(() => { - if (!connectionLostNotificationIdRef.current) return; - removeNotification(connectionLostNotificationIdRef.current); - connectionLostNotificationIdRef.current = null; + if (!notificationIdRef.current) return; + removeNotification(notificationIdRef.current); + notificationIdRef.current = null; + reasonRef.current = null; }, [removeNotification]); /** - * Dismissal is scoped to the mount, not to the subscription below. + * Dismissal is scoped to the mount, not to the subscriptions below. * - * The subscription's dependencies change for reasons that have nothing to do with the connection — - * `t` is replaced when `Streami18n.init()` resolves, asynchronously. When dismissal was part of - * that effect's cleanup, a socket dropping before init finished had its notification published and - * then immediately removed, leaving no banner on an offline app launch or behind a captive portal. + * Their dependencies change for reasons that have nothing to do with the connection — `t` is + * replaced when `Streami18n.init()` resolves, asynchronously. When dismissal was part of that + * effect's cleanup, a socket dropping before init finished had its notification published and then + * immediately removed, leaving no banner on an offline app launch or behind a captive portal. */ useEffect(() => dismissConnectionLostNotification, [dismissConnectionLostNotification]); useEffect(() => { if (!t || !client) return; - const handleConnectionChanged = ({ - connection, - online, - }: EventPayload<'connection.changed'>) => { - // Narrowed to the socket, which is what this hook has always reported — the notification type - // says `network`, but the fact behind it is the WebSocket. Now that the event also arrives for - // the device's network, the guard is what keeps that unchanged rather than silently doubling. - // Whether a lost *network* deserves its own notification is a separate question. - if (connection !== 'ws') return; + // Keyed by `ConnectionType` rather than a local union, so a connection type added to the client + // breaks `yarn build` here (`tsconfig.lib.json`) until someone decides what the banner should say + // about it. Verified by widening the type: `Property 'sse' is missing`. Note `yarn types` does + // *not* catch it — without `strictNullChecks` the absent key is `undefined`, which is assignable + // to `string`. + const messages: Record = { + network: t('chat.reportLostConnection.waitingNetwork.text', 'Waiting for network…'), + ws: t('chat.reportLostConnection.reconnecting.text', 'Reconnecting…'), + }; + + const show = (reason: ConnectionType) => { + if (reasonRef.current === reason) return; + // Replaced rather than left alone: a network drop while "Reconnecting…" is showing needs the + // more specific message. + dismissConnectionLostNotification(); + reasonRef.current = reason; + notificationIdRef.current = addSystemNotification({ + duration: 0, + emitter: 'Chat', + message: messages[reason], + severity: 'loading', + // One type for both messages, deliberately. Consumers filter banners on it — the SDK's own + // cookbook recipe does — so splitting it would silently stop those filters seeing the socket + // case. The `network` in the name is historical; the message is what was wrong. + type: 'system:network:connection:lost', + }); + }; - if (!online) { - if (connectionLostNotificationIdRef.current) return; + if (socketOnlineRef.current === null) { + socketOnlineRef.current = client.wsConnection.isOnline; + } - connectionLostNotificationIdRef.current = addSystemNotification({ - duration: 0, - emitter: 'Chat', - message: t( - 'chat.reportLostConnection.waitingNetwork.text', - 'Waiting for network…', - ), - severity: 'loading', - type: 'system:network:connection:lost', - }); - return; - } + // `=== false` for the network, never `!networkOnline`: `undefined` means nobody has told us, and + // on a host with no registrar that must not read as offline. The socket's is a plain boolean. + let networkOnline = client.networkConnection.isOnline; + const sync = () => { + if (networkOnline === false) return show('network'); + if (!socketOnlineRef.current) return show('ws'); dismissConnectionLostNotification(); }; - const subscription = client.on('connection.changed', handleConnectionChanged); + const unsubscribeNetwork = client.networkConnection.state.subscribeWithSelector( + ({ isOnline }) => ({ isOnline }), + ({ isOnline }) => { + networkOnline = isOnline; + sync(); + }, + ); + + // The socket half stays on the event, not on `client.wsConnection.state`, because the event is + // debounced by five seconds on the way down and dropped entirely if the socket returns inside + // that window. Subscribing to the store would publish the raw edge and strobe the banner on a + // brief flap. + const { unsubscribe: unsubscribeSocket } = client.on( + 'connection.changed', + (event) => { + if (event.connection !== 'ws') return; + socketOnlineRef.current = event.online; + sync(); + }, + ); - return subscription.unsubscribe; + // Read the current state rather than waiting for a transition — a client already offline when + // this mounts showed nothing at all before. + sync(); + + return () => { + unsubscribeNetwork(); + unsubscribeSocket(); + }; }, [addSystemNotification, client, dismissConnectionLostNotification, t]); }; diff --git a/src/i18n/__tests__/catalog.fixture.json b/src/i18n/__tests__/catalog.fixture.json index be04a7336..77db7adfa 100644 --- a/src/i18n/__tests__/catalog.fixture.json +++ b/src/i18n/__tests__/catalog.fixture.json @@ -184,6 +184,7 @@ "channelListItem.video.ariaLabel": "video", "channelListItem.voiceMessage.ariaLabel": "voice message", "channelListItem.voted.text": "📊 {{votedBy}} voted: {{pollOptionText}}", + "chat.reportLostConnection.reconnecting.text": "Reconnecting…", "chat.reportLostConnection.waitingNetwork.text": "Waiting for network…", "command.ban.args": "[@username] [text]", "command.ban.description": "Ban a user", diff --git a/src/i18n/keys.ts b/src/i18n/keys.ts index 7da3f379e..94e9d068d 100644 --- a/src/i18n/keys.ts +++ b/src/i18n/keys.ts @@ -195,6 +195,7 @@ export type TranslationCatalog = { 'channelListItem.video.ariaLabel': 'video'; 'channelListItem.voiceMessage.ariaLabel': 'voice message'; 'channelListItem.voted.text': '📊 {{votedBy}} voted: {{pollOptionText}}'; + 'chat.reportLostConnection.reconnecting.text': 'Reconnecting…'; 'chat.reportLostConnection.waitingNetwork.text': 'Waiting for network…'; 'command.ban.args': '[@username] [text]'; 'command.ban.description': 'Ban a user'; From 60dd9b981836a609593e34a19ee26bbf82c8784a Mon Sep 17 00:00:00 2001 From: martincupela Date: Thu, 10 Sep 2026 10:46:37 +0200 Subject: [PATCH 7/7] chore(examples): add a connection dev panel to the vite example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driving the two connection facts apart by hand is awkward, and the obvious tool is the wrong one: DevTools' "Offline" checkbox takes down `navigator.onLine` *and* the socket, which is the one combination that always worked. What needs exercising is a dead socket on a live network — a server close, an expired token, a health-check timeout — because that is the case the offline banner used to describe as "Waiting for network…". Two toggles, showing both facts and flipping each independently. Hidden behind a sidebar button that mirrors the theme and RTL toggles, backed by a `devTools.connectionPanel` setting that defaults to off. Both toggles simulate rather than sever, and the socket one writes the store *and* dispatches `connection.changed`, because that is what the real socket does: the store carries the raw state, the event is the announcement the banner listens to. Closing the real socket is no use for a toggle — it reconnects on its own within a second or two, so it would flip back by itself — and the one thing that does hold, `client.closeConnection()`, deliberately dispatches no event, so no banner appears. The component's doc gives the console one-liner for the genuine path, including the five-second announce delay the toggles cannot show. The German and Italian dictionaries here are complete by type assertion, so the new `chat.reportLostConnection.reconnecting.text` key needed translating in both — the assertion fails the build naming any key left out. Earned its keep immediately: it surfaced a regression where every watched request went out without a `connection_id`. --- examples/vite/src/App.tsx | 5 + examples/vite/src/AppSettings/AppSettings.tsx | 26 +++++ examples/vite/src/AppSettings/state.ts | 10 ++ .../ConnectionDevPanel.scss | 52 +++++++++ .../ConnectionDevPanel/ConnectionDevPanel.tsx | 100 ++++++++++++++++++ examples/vite/src/i18n/de.ts | 1 + examples/vite/src/i18n/it.ts | 1 + examples/vite/src/icons.tsx | 13 +++ 8 files changed, 208 insertions(+) create mode 100644 examples/vite/src/ConnectionDevPanel/ConnectionDevPanel.scss create mode 100644 examples/vite/src/ConnectionDevPanel/ConnectionDevPanel.tsx diff --git a/examples/vite/src/App.tsx b/examples/vite/src/App.tsx index ea1c7170e..f7ede4014 100644 --- a/examples/vite/src/App.tsx +++ b/examples/vite/src/App.tsx @@ -73,6 +73,7 @@ import { resolveSingleChannel, SingleChannelModal, } from './SingleChannel/SingleChannelApp.tsx'; +import { ConnectionDevPanel } from './ConnectionDevPanel/ConnectionDevPanel.tsx'; import { SystemNotification } from './SystemNotification/SystemNotification.tsx'; import { chatViewSelectorItemSet } from './Sidebar/ChatViewSelectorItemSet.tsx'; import { @@ -263,6 +264,9 @@ const App = () => { channelCid: state.layout.channelCid, })); const { mode: themeMode } = useAppSettingsSelector((state) => state.theme); + const { connectionPanel: connectionPanelVisible } = useAppSettingsSelector( + (state) => state.devTools, + ); const initialChannelId = useMemo(() => getInitialChannelIdFromUrl(), []); const initialChatView = useMemo(() => getInitialChatViewFromUrl(), []); const initialThreadId = useMemo(() => getInitialThreadIdFromUrl(), []); @@ -546,6 +550,7 @@ const App = () => { ref={appLayoutRef} style={initialAppLayoutStyle} > + {connectionPanelVisible && }
diff --git a/examples/vite/src/AppSettings/AppSettings.tsx b/examples/vite/src/AppSettings/AppSettings.tsx index 8a739ee29..234235e12 100644 --- a/examples/vite/src/AppSettings/AppSettings.tsx +++ b/examples/vite/src/AppSettings/AppSettings.tsx @@ -26,6 +26,7 @@ import { ReactionsTab } from './tabs/Reactions'; import { SidebarTab } from './tabs/Sidebar'; import { appSettingsStore, useAppSettingsState } from './state'; import { + IconConnection, IconGear, IconMoon, IconSidebar, @@ -159,6 +160,30 @@ const SidebarThemeToggle = ({ iconOnly = true }: { iconOnly?: boolean }) => { ); }; +const SidebarConnectionPanelToggle = ({ iconOnly = true }: { iconOnly?: boolean }) => { + const { devTools } = useAppSettingsState(); + const { connectionPanel } = devTools; + + return ( + + appSettingsStore.partialNext({ + devTools: { ...devTools, connectionPanel: !connectionPanel }, + }) + } + role='switch' + text={connectionPanel ? 'Hide connection panel' : 'Connection panel'} + /> + ); +}; + const SidebarRtlToggle = ({ iconOnly = true }: { iconOnly?: boolean }) => { const { theme, @@ -203,6 +228,7 @@ export const AppSettings = ({ iconOnly = true }: { iconOnly?: boolean }) => { return (
+ diff --git a/examples/vite/src/AppSettings/state.ts b/examples/vite/src/AppSettings/state.ts index d8e992e0f..d3671494b 100644 --- a/examples/vite/src/AppSettings/state.ts +++ b/examples/vite/src/AppSettings/state.ts @@ -13,6 +13,12 @@ export type ChatViewSettingsState = { iconOnly: boolean; }; +/** Dev-only affordances that would not ship in an application. */ +export type DevToolsSettingsState = { + /** Shows the panel that drives the network and WebSocket facts independently. */ + connectionPanel: boolean; +}; + export type ThemeSettingsState = { direction: 'ltr' | 'rtl'; mode: 'dark' | 'light'; @@ -85,6 +91,7 @@ export type LayoutSettingsState = { export type AppSettingsState = { channelDetail: ChannelDetailSettingsState; chatView: ChatViewSettingsState; + devTools: DevToolsSettingsState; language: LanguageSettingsState; layout: LayoutSettingsState; messageActions: MessageActionsSettingsState; @@ -113,6 +120,9 @@ const isRecord = (value: unknown): value is Record => typeof value === 'object' && value !== null; const defaultAppSettingsState: AppSettingsState = { + devTools: { + connectionPanel: false, + }, channelDetail: { modal: { channelMembersView: { diff --git a/examples/vite/src/ConnectionDevPanel/ConnectionDevPanel.scss b/examples/vite/src/ConnectionDevPanel/ConnectionDevPanel.scss new file mode 100644 index 000000000..2d4ad3544 --- /dev/null +++ b/examples/vite/src/ConnectionDevPanel/ConnectionDevPanel.scss @@ -0,0 +1,52 @@ +.connection-dev-panel { + align-items: center; + background: var(--str-chat__secondary-surface-color, #f7f7f8); + border-bottom: 1px solid var(--str-chat__surface-color, #e3e5e8); + display: flex; + flex-wrap: wrap; + font-family: monospace; + font-size: 11px; + gap: 10px; + padding: 6px 12px; + + &__label { + opacity: 0.6; + text-transform: uppercase; + } + + &__toggle { + align-items: center; + background: #fff; + border: 1px solid var(--str-chat__surface-color, #d0d3d8); + border-radius: 3px; + cursor: pointer; + display: flex; + font-family: inherit; + font-size: inherit; + gap: 8px; + padding: 3px 8px; + + // `false` is the only definite negative. `undefined` means unknown and gets its own colour, so + // the state that must not read as offline does not look like it. + &[data-state='false'] { + background: #ffd7d7; + } + + &[data-state='true'] { + background: #d7f5dd; + } + + &[data-state='undefined'] { + background: #ffe9c7; + } + } + + &__action { + opacity: 0.55; + } + + &__hint { + margin-left: auto; + opacity: 0.5; + } +} diff --git a/examples/vite/src/ConnectionDevPanel/ConnectionDevPanel.tsx b/examples/vite/src/ConnectionDevPanel/ConnectionDevPanel.tsx new file mode 100644 index 000000000..2beaa4a46 --- /dev/null +++ b/examples/vite/src/ConnectionDevPanel/ConnectionDevPanel.tsx @@ -0,0 +1,100 @@ +import { + useChatContext, + useNetworkConnectionState, + useWSConnectionState, +} from 'stream-chat-react'; + +import './ConnectionDevPanel.scss'; + +/** + * Drives the two connection facts independently, so the pair that disagrees can actually be seen. + * + * DevTools' "Offline" checkbox is no use for this: it takes down `navigator.onLine` **and** the + * socket, which is the one combination that always worked. What needs exercising is a dead socket on + * a live network — a server close, an expired token, a health-check timeout — because that is the case + * the banner used to describe as "Waiting for network…". + * + * **Both toggles simulate rather than sever.** The socket one writes + * `client.wsConnection.state` *and* dispatches `connection.changed`, which is what the real socket + * does — the store carries the raw state, the event is the announcement. Closing the real socket is + * no good for a toggle: `StableWSConnection` reconnects on its own within a second or two, so it + * would flip back by itself, and the one thing that does hold — `client.closeConnection()` — + * deliberately dispatches no event, so no banner appears. + * + * The one thing this cannot show is the five-second delay the real event carries on the way down. + * + * For the genuine path, including the five-second announce delay on the way down, close the socket + * from the console instead: + * + * ```js + * client.wsConnection.connection.ws.close() + * ``` + * + * Dev-only. Nothing here belongs in an application: `setStatus` is for a platform listener rather + * than a button. + */ +export const ConnectionDevPanel = () => { + const { client } = useChatContext(); + const { isOnline: networkOnline } = useNetworkConnectionState() ?? {}; + const { isOnline: socketOnline, connectionId } = useWSConnectionState() ?? {}; + + if (!client) return null; + + return ( + + ); +}; diff --git a/examples/vite/src/i18n/de.ts b/examples/vite/src/i18n/de.ts index da3af8b3e..e2cac8e7b 100644 --- a/examples/vite/src/i18n/de.ts +++ b/examples/vite/src/i18n/de.ts @@ -259,6 +259,7 @@ export const deTranslations = { 'channelListItem.video.ariaLabel': 'Video', 'channelListItem.voiceMessage.ariaLabel': 'Sprachnachricht', 'channelListItem.voted.text': '📊 {{votedBy}} hat abgestimmt: {{pollOptionText}}', + 'chat.reportLostConnection.reconnecting.text': 'Verbindung wird wiederhergestellt…', 'chat.reportLostConnection.waitingNetwork.text': 'Warte auf Netzwerk…', 'command.ban.args': '[@benutzername] [text]', 'command.ban.description': 'Einen Benutzer sperren', diff --git a/examples/vite/src/i18n/it.ts b/examples/vite/src/i18n/it.ts index 28d0ccb4b..ac6f7e4c1 100644 --- a/examples/vite/src/i18n/it.ts +++ b/examples/vite/src/i18n/it.ts @@ -248,6 +248,7 @@ export const itTranslations = { 'channelListItem.video.ariaLabel': 'video', 'channelListItem.voiceMessage.ariaLabel': 'messaggio vocale', 'channelListItem.voted.text': '📊 {{votedBy}} ha votato: {{pollOptionText}}', + 'chat.reportLostConnection.reconnecting.text': 'Riconnessione…', 'chat.reportLostConnection.waitingNetwork.text': 'In attesa della rete…', 'command.ban.args': '[@nomeutente] [testo]', 'command.ban.description': 'Banna un utente', diff --git a/examples/vite/src/icons.tsx b/examples/vite/src/icons.tsx index 1b3ecbf38..cc97e568e 100644 --- a/examples/vite/src/icons.tsx +++ b/examples/vite/src/icons.tsx @@ -12,6 +12,19 @@ export const IconSidebar = createIcon( />, ); +/** Two arcs and a dot — a signal/reachability mark, for the connection dev panel toggle. */ +export const IconConnection = createIcon( + 'IconConnection', + , +); + export const IconGear = createIcon( 'IconGear', ,