From 56d73d1dfc2fdfaef8d842996df32a52cba04442 Mon Sep 17 00:00:00 2001 From: "Timo K." Date: Wed, 11 Nov 2026 19:11:35 +0100 Subject: [PATCH] Track the client's connection in a hook of its own Whether the client is disconnected is a fact about the client's sync, not about the session, yet the provider owned the sync listener and put the flag on the context state, so the banner had to reach through the whole `ClientState` to read it. `useClientDisconnected` listens on whichever client is signed in, tagging its verdict with that client so a swapped client does not inherit the old one's before its first sync. The avatar, which only needs the client, takes it from `useClient` in the same spirit: consumers read the narrowest hook that answers them. Co-Authored-By: Claude Fable 5.1 --- src/Avatar.test.tsx | 1 - src/Avatar.tsx | 16 +++---- src/ClientContext.test.tsx | 30 +++++++++++- src/ClientContext.tsx | 61 +++++++++++------------- src/DisconnectedBanner.tsx | 10 +--- src/settings/useSubmitRageshake.test.tsx | 1 - 6 files changed, 66 insertions(+), 53 deletions(-) diff --git a/src/Avatar.test.tsx b/src/Avatar.test.tsx index dcac076b9..3d1cb9862 100644 --- a/src/Avatar.test.tsx +++ b/src/Avatar.test.tsx @@ -29,7 +29,6 @@ const TestComponent: FC< = ({ style, ...props }) => { - const clientState = useClientState(); + const { client } = useClient(); const hostBridge = useHostBridge(); const sizePx = useMemo( @@ -87,7 +87,7 @@ export const Avatar: FC = ({ const [avatarUrl, setAvatarUrl] = useState(undefined); - // In theory, a change in `clientState` or `sizePx` could run extra media + // In theory, a change in `client` or `sizePx` could run extra media // downloads, but in practice they should be stable long before this code runs. useEffect(() => { if (!src) { @@ -99,12 +99,8 @@ export const Avatar: FC = ({ if (hostBridge.downloadMedia) { blob = hostBridge.downloadMedia(src); - } else if ( - clientState?.state === "valid" && - clientState.authenticated?.client && - sizePx - ) { - blob = getAvatarFromServer(clientState.authenticated.client, src, sizePx); + } else if (client && sizePx) { + blob = getAvatarFromServer(client, src, sizePx); } else { setAvatarUrl(undefined); return; @@ -133,7 +129,7 @@ export const Avatar: FC = ({ URL.revokeObjectURL(objectUrl); } }; - }, [clientState, hostBridge, src, sizePx]); + }, [client, hostBridge, src, sizePx]); return ( expect(container.textContent).toBe("unauthenticated")); }); + +test("reports the client as disconnected while its sync fails to connect", () => { + const client = mockClient(); + const { result } = renderHook(() => useClientDisconnected(), { + wrapper: ({ children }) => ( + + <>{children} + + ), + }); + expect(result.current).toBe(false); + + const [, onSync] = vi.mocked(client.on).mock.calls[0] as [ + unknown, + (state: string, old: null, data?: { error?: Error }) => void, + ]; + const connectionError = Object.assign(new Error("offline"), { + name: "ConnectionError", + }); + act(() => onSync("ERROR", null, { error: connectionError })); + expect(result.current).toBe(true); + // Any other failure is not a connection problem + act(() => onSync("ERROR", null, { error: new Error("rate limited") })); + expect(result.current).toBe(false); + act(() => onSync("SYNCING", null)); + expect(result.current).toBe(false); +}); diff --git a/src/ClientContext.tsx b/src/ClientContext.tsx index 0d0aee56c..97ee1f2e1 100644 --- a/src/ClientContext.tsx +++ b/src/ClientContext.tsx @@ -41,9 +41,6 @@ export type ClientState = ValidClientState | ErrorState; export type ValidClientState = { state: "valid"; authenticated?: AuthenticatedClient; - // 'Disconnected' rather than 'connected' because it tracks specifically - // whether the client is supposed to be connected but is not - disconnected: boolean; setClient: (client: MatrixClient, session: Session) => void; }; @@ -124,6 +121,33 @@ export function useClientLegacy(): { }; } +/** + * Whether the signed-in client is supposed to be connected but is not. + */ +export function useClientDisconnected(): boolean { + const { client } = useClient(); + // Tagged with the client it describes, so that a swapped client does not + // inherit the old one's verdict before its first sync + const [disconnected, setDisconnected] = useState< + [MatrixClient, boolean] | null + >(null); + + useEffect(() => { + if (client === undefined) return; + const onSync = ( + syncState: SyncState, + _old: SyncState | null, + data?: ISyncStateData, + ): void => setDisconnected([client, clientIsDisconnected(syncState, data)]); + client.on(ClientEvent.Sync, onSync); + return (): void => { + client.removeListener(ClientEvent.Sync, onSync); + }; + }, [client]); + + return disconnected !== null && disconnected[0] === client && disconnected[1]; +} + const loadChannel = "BroadcastChannel" in window ? new BroadcastChannel("load") : null; @@ -300,8 +324,6 @@ export const ClientProvider: FC = ({ children, client, loadClient }) => { }, [initClientState?.client, setAlreadyOpenedErr]), ); - const [isDisconnected, setIsDisconnected] = useState(false); - const state: ClientState | undefined = useMemo(() => { if (alreadyOpenedErr) { return { state: "error", error: alreadyOpenedErr }; @@ -323,23 +345,8 @@ export const ClientProvider: FC = ({ children, client, loadClient }) => { state: "valid", authenticated, setClient, - disconnected: isDisconnected, }; - }, [ - alreadyOpenedErr, - changePassword, - initClientState, - logout, - setClient, - isDisconnected, - ]); - - const onSync = useCallback( - (state: SyncState, _old: SyncState | null, data?: ISyncStateData) => { - setIsDisconnected(clientIsDisconnected(state, data)); - }, - [], - ); + }, [alreadyOpenedErr, changePassword, initClientState, logout, setClient]); useEffect(() => { if (!initClientState) { @@ -351,17 +358,7 @@ export const ClientProvider: FC = ({ children, client, loadClient }) => { if (PosthogAnalytics.hasInstance()) PosthogAnalytics.instance.onLoginStatusChanged(); - - if (initClientState.client) { - initClientState.client.on(ClientEvent.Sync, onSync); - } - - return (): void => { - if (initClientState.client) { - initClientState.client.removeListener(ClientEvent.Sync, onSync); - } - }; - }, [initClientState, onSync]); + }, [initClientState]); if (alreadyOpenedErr) { return ; diff --git a/src/DisconnectedBanner.tsx b/src/DisconnectedBanner.tsx index 57d657958..e9c269422 100644 --- a/src/DisconnectedBanner.tsx +++ b/src/DisconnectedBanner.tsx @@ -10,7 +10,7 @@ import { type FC, type HTMLAttributes, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; import styles from "./DisconnectedBanner.module.css"; -import { type ValidClientState, useClientState } from "./ClientContext"; +import { useClientDisconnected } from "./ClientContext"; interface Props extends HTMLAttributes { children?: ReactNode; @@ -23,13 +23,7 @@ export const DisconnectedBanner: FC = ({ ...rest }) => { const { t } = useTranslation(); - const clientState = useClientState(); - let shouldShowBanner = false; - - if (clientState?.state === "valid") { - const validClientState = clientState as ValidClientState; - shouldShowBanner = validClientState.disconnected; - } + const shouldShowBanner = useClientDisconnected(); return ( <> diff --git a/src/settings/useSubmitRageshake.test.tsx b/src/settings/useSubmitRageshake.test.tsx index 60b55bbd6..a53398d39 100644 --- a/src/settings/useSubmitRageshake.test.tsx +++ b/src/settings/useSubmitRageshake.test.tsx @@ -77,7 +77,6 @@ function renderWithMockClient(