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 <noreply@anthropic.com>

src/Avatar.test.tsx

@@ -29,7 +29,6 @@ const TestComponent: FC<
     <ClientContextProvider
       value={{
         state: "valid",
-        disconnected: false,
         setClient: vi.fn(),
         authenticated: {
           client,

src/Avatar.tsx

@@ -15,7 +15,7 @@ import {
 import { Avatar as CompoundAvatar } from "@vector-im/compound-web";
 import { type MatrixClient } from "matrix-js-sdk";
 
-import { useClientState } from "./ClientContext";
+import { useClient } from "./ClientContext";
 import { useHostBridge } from "./HostBridge";
 
 export enum Size {
@@ -74,7 +74,7 @@ export const Avatar: FC<Props> = ({
   style,
   ...props
 }) => {
-  const clientState = useClientState();
+  const { client } = useClient();
   const hostBridge = useHostBridge();
 
   const sizePx = useMemo(
@@ -87,7 +87,7 @@ export const Avatar: FC<Props> = ({
 
   const [avatarUrl, setAvatarUrl] = useState<string | undefined>(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<Props> = ({
 
     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<Props> = ({
         URL.revokeObjectURL(objectUrl);
       }
     };
-  }, [clientState, hostBridge, src, sizePx]);
+  }, [client, hostBridge, src, sizePx]);
 
   return (
     <CompoundAvatar

src/ClientContext.test.tsx

@@ -6,7 +6,7 @@ Please see LICENSE in the repository root for full details.
 */
 
 import { expect, test, vi } from "vitest";
-import { render, waitFor } from "@testing-library/react";
+import { act, render, renderHook, waitFor } from "@testing-library/react";
 import { BrowserRouter } from "react-router-dom";
 import { type MatrixClient } from "matrix-js-sdk";
 import { type FC } from "react";
@@ -14,6 +14,7 @@ import { type FC } from "react";
 import {
   type ClientLoader,
   ClientProvider,
+  useClientDisconnected,
   useClientState,
 } from "./ClientContext";
 
@@ -132,3 +133,30 @@ test("has no one signed in when given neither a client nor a way to load one", a
 
   await waitFor(() => 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 }) => (
+      <ClientProvider client={client}>
+        <>{children}</>
+      </ClientProvider>
+    ),
+  });
+  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);
+});

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<Props> = ({ 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<Props> = ({ 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<Props> = ({ 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 <ErrorPage error={alreadyOpenedErr} />;

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<HTMLElement> {
   children?: ReactNode;
@@ -23,13 +23,7 @@ export const DisconnectedBanner: FC<Props> = ({
   ...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 (
     <>

src/settings/useSubmitRageshake.test.tsx

@@ -77,7 +77,6 @@ function renderWithMockClient(
     <ClientContextProvider
       value={{
         state: "valid",
-        disconnected: false,
         setClient: vi.fn(),
         authenticated: {
           client,