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(