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', , diff --git a/src/components/Channel/Channel.tsx b/src/components/Channel/Channel.tsx index 6a4b7b944..724705a6a 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, @@ -136,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 @@ -146,33 +144,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 - // 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. - // - // `recoverState` dispatches this only after re-querying the active channels, so the rest of the - // channel state is already fresh by now. - 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; @@ -306,8 +277,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); client.on('user.messages.deleted', handleEvent); @@ -318,8 +287,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); }; // 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 7b11178cf..70dfed8c1 100644 --- a/src/components/Channel/__tests__/Channel.test.tsx +++ b/src/components/Channel/__tests__/Channel.test.tsx @@ -406,21 +406,46 @@ 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 () => { + dispatchConnectionChangedEvent(chatClient, true, 'ws'); + await new Promise((resolve) => setTimeout(resolve, 50)); + }); + + expect(reloadSpy).toHaveBeenCalledTimes(1); + }); + + 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); await Promise.resolve(); }); - await waitFor(() => expect(reloadSpy).toHaveBeenCalledTimes(1)); + expect(reloadSpy).not.toHaveBeenCalled(); }); it('does not reload a channel that is pending disposal', async () => { @@ -439,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(); }); }); diff --git a/src/components/Chat/__tests__/Chat.test.tsx b/src/components/Chat/__tests__/Chat.test.tsx index 3c5d37644..d35b6285e 100644 --- a/src/components/Chat/__tests__/Chat.test.tsx +++ b/src/components/Chat/__tests__/Chat.test.tsx @@ -376,37 +376,148 @@ describe('Chat', () => { }); describe('connection notifications', () => { - it('publishes and removes system connection-lost notification on connection changes', async () => { - const client = getTestClient(); - let connectionLostNotification; - + 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(
, ); - - expect(client.notifications.notifications).toHaveLength(0); - - act(() => dispatchConnectionChangedEvent(client, false)); - await waitFor(() => { - connectionLostNotification = client.notifications.notifications.find( + const chatNotifications = () => + client.notifications.notifications.filter( (notification) => notification.origin.emitter === 'Chat', ); - expect(connectionLostNotification).toBeDefined(); + + // Deliberately not awaiting anything first — the drop lands inside the init window. + act(() => dispatchConnectionChangedEvent(client, false, 'ws')); + 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(connectionLostNotification.message).toBe('Waiting for network…'); - expect(connectionLostNotification.tags).toEqual(['system']); + expect(chatNotifications()).toHaveLength(1); + }); - act(() => dispatchConnectionChangedEvent(client, true)); - await waitFor(() => { - expect( - client.notifications.notifications.find( - (notification) => notification.origin.emitter === 'Chat', - ), - ).toBeUndefined(); + /** + * 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( + +
+ , + ); + // 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, 'ws')); + + 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( + +
+ , + ); + + act(() => dispatchConnectionChangedEvent(client, false, 'ws')); + await waitFor(() => + expect(chatNotificationsOf(client)[0].message).toBe('Reconnecting…'), + ); + + 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 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( + +
+ , + ); + + await waitFor(() => expect(chatNotificationsOf(client)).toHaveLength(1)); + expect(chatNotificationsOf(client)[0].message).toBe('Waiting for network…'); + }); + + it('clears on recovery', async () => { + const client = await getTestClientWithUser(); + render( + +
+ , + ); + + act(() => dispatchConnectionChangedEvent(client, false, 'ws')); + await waitFor(() => expect(chatNotificationsOf(client)).toHaveLength(1)); + + 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/__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/useReportLostConnectionSystemNotification.ts b/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts index e916f29da..447724cb3 100644 --- a/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts +++ b/src/components/Chat/hooks/useReportLostConnectionSystemNotification.ts @@ -1,54 +1,140 @@ -import { useEffect, useRef } from 'react'; -import type { EventPayload } from 'stream-chat'; +import { useCallback, useEffect, useRef } from 'react'; +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 (!notificationIdRef.current) return; + removeNotification(notificationIdRef.current); + notificationIdRef.current = null; + reasonRef.current = null; + }, [removeNotification]); + + /** + * Dismissal is scoped to the mount, not to the subscriptions below. + * + * 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 dismissConnectionLostNotification = () => { - if (!connectionLostNotificationIdRef.current) return; - removeNotification(connectionLostNotificationIdRef.current); - connectionLostNotificationIdRef.current = null; + // 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', + }); }; - const handleConnectionChanged = ({ online }: EventPayload<'connection.changed'>) => { - if (!online) { - if (connectionLostNotificationIdRef.current) return; - - connectionLostNotificationIdRef.current = addSystemNotification({ - duration: 0, - emitter: 'Chat', - message: t( - 'chat.reportLostConnection.waitingNetwork.text', - 'Waiting for network…', - ), - severity: 'loading', - type: 'system:network:connection:lost', - }); - return; - } + if (socketOnlineRef.current === null) { + socketOnlineRef.current = client.wsConnection.isOnline; + } + // `=== 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(); + }, + ); + + // Read the current state rather than waiting for a transition — a client already offline when + // this mounts showed nothing at all before. + sync(); return () => { - subscription.unsubscribe(); - dismissConnectionLostNotification(); + unsubscribeNetwork(); + unsubscribeSocket(); }; - }, [addSystemNotification, client, removeNotification, t]); + }, [addSystemNotification, client, dismissConnectionLostNotification, t]); }; 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/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'; 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', }), ); 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(); });