Avoid reactivity bugs in how we track external state

src/room/GroupCallView.test.tsx

@@ -29,6 +29,7 @@ import { useAudioContext } from "../useAudioContext";
 import { ActiveCall } from "./InCallView";
 import {
   flushPromises,
+  mockEmitter,
   mockMatrixRoom,
   mockMatrixRoomMember,
   mockRtcMembership,
@@ -129,6 +130,7 @@ function createGroupCallView(
     getMxcAvatarUrl: () => null,
     getCanonicalAlias: () => null,
     currentState: {
+      ...mockEmitter(),
       getJoinRule: () => JoinRule.Invite,
     } as Partial<RoomState> as RoomState,
   });

src/room/useActiveFocus.ts

@@ -9,11 +9,13 @@ import {
   type MatrixRTCSession,
   MatrixRTCSessionEvent,
 } from "matrix-js-sdk/lib/matrixrtc";
-import { useCallback, useEffect, useState } from "react";
+import { useCallback, useEffect, useRef } from "react";
 import { deepCompare } from "matrix-js-sdk/lib/utils";
 import { logger } from "matrix-js-sdk/lib/logger";
 import { type LivekitFocus, isLivekitFocus } from "matrix-js-sdk/lib/matrixrtc";
 
+import { useTypedEventEmitterState } from "../useEvents";
+
 /**
  * Gets the currently active (livekit) focus for a MatrixRTC session
  * This logic is specific to livekit foci where the whole call must use one
@@ -22,38 +24,27 @@ import { type LivekitFocus, isLivekitFocus } from "matrix-js-sdk/lib/matrixrtc";
 export function useActiveLivekitFocus(
   rtcSession: MatrixRTCSession,
 ): LivekitFocus | undefined {
-  const [activeFocus, setActiveFocus] = useState(() => {
-    const f = rtcSession.getActiveFocus();
-    // Only handle foci with type="livekit" for now.
-    return !!f && isLivekitFocus(f) ? f : undefined;
-  });
+  const activeFocus = useTypedEventEmitterState(
+    rtcSession,
+    MatrixRTCSessionEvent.MembershipsChanged,
+    useCallback(() => {
+      const f = rtcSession.getActiveFocus();
+      // Only handle foci with type="livekit" for now.
+      return !!f && isLivekitFocus(f) ? f : undefined;
+    }, [rtcSession]),
+  );
 
-  const onMembershipsChanged = useCallback(() => {
-    const newActiveFocus = rtcSession.getActiveFocus();
-    if (!!newActiveFocus && !isLivekitFocus(newActiveFocus)) return;
-    if (!deepCompare(activeFocus, newActiveFocus)) {
+  const prevActiveFocus = useRef(activeFocus);
+  useEffect(() => {
+    if (!deepCompare(activeFocus, prevActiveFocus.current)) {
       const oldestMembership = rtcSession.getOldestMembership();
       logger.warn(
         `Got new active focus from membership: ${oldestMembership?.sender}/${oldestMembership?.deviceId}.
-        Updating focus (focus switch) from ${JSON.stringify(activeFocus)} to ${JSON.stringify(newActiveFocus)}`,
+        Updated focus (focus switch) from ${JSON.stringify(prevActiveFocus.current)} to ${JSON.stringify(activeFocus)}`,
       );
-      setActiveFocus(newActiveFocus);
+      prevActiveFocus.current = activeFocus;
     }
   }, [activeFocus, rtcSession]);
 
-  useEffect(() => {
-    rtcSession.on(
-      MatrixRTCSessionEvent.MembershipsChanged,
-      onMembershipsChanged,
-    );
-
-    return (): void => {
-      rtcSession.off(
-        MatrixRTCSessionEvent.MembershipsChanged,
-        onMembershipsChanged,
-      );
-    };
-  });
-
   return activeFocus;
 }

src/room/useRoomName.ts

@@ -1,18 +1,19 @@
 /*
-Copyright 2023, 2024 New Vector Ltd.
+Copyright 2025 New Vector Ltd.
 
 SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 Please see LICENSE in the repository root for full details.
 */
 
 import { type Room, RoomEvent } from "matrix-js-sdk";
-import { useState } from "react";
+import { useCallback } from "react";
 
-import { useTypedEventEmitter } from "../useEvents";
+import { useTypedEventEmitterState } from "../useEvents";
 
 export function useRoomName(room: Room): string {
-  const [, setNumUpdates] = useState(0);
-  // Whenever the name changes, force an update
-  useTypedEventEmitter(room, RoomEvent.Name, () => setNumUpdates((n) => n + 1));
-  return room.name;
+  return useTypedEventEmitterState(
+    room,
+    RoomEvent.Name,
+    useCallback(() => room.name, [room]),
+  );
 }

src/room/useRoomState.ts

@@ -5,25 +5,33 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 Please see LICENSE in the repository root for full details.
 */
 
-import { useCallback, useMemo, useState } from "react";
-import { type RoomState, RoomStateEvent, type Room } from "matrix-js-sdk";
+import { useCallback } from "react";
+import {
+  type RoomState,
+  RoomStateEvent,
+  type Room,
+  RoomEvent,
+} from "matrix-js-sdk";
 
-import { useTypedEventEmitter } from "../useEvents";
+import { useTypedEventEmitterState } from "../useEvents";
 
 /**
  * A React hook for values computed from room state.
  * @param room The room.
  * @param f A mapping from the current room state to the computed value.
  * @returns The computed value.
  */
-export const useRoomState = <T>(room: Room, f: (state: RoomState) => T): T => {
-  const [numUpdates, setNumUpdates] = useState(0);
-  useTypedEventEmitter(
+export function useRoomState<T>(room: Room, f: (state: RoomState) => T): T {
+  // TODO: matrix-js-sdk says that Room.currentState is deprecated, but it's not
+  // clear how to reactively track the current state of the room without it
+  const currentState = useTypedEventEmitterState(
     room,
+    RoomEvent.CurrentStateUpdated,
+    useCallback(() => room.currentState, [room]),
+  );
+  return useTypedEventEmitterState(
+    currentState,
     RoomStateEvent.Update,
-    useCallback(() => setNumUpdates((n) => n + 1), [setNumUpdates]),
+    useCallback(() => f(currentState), [f, currentState]),
   );
-  // We want any change to the update counter to trigger an update here
-  // eslint-disable-next-line react-hooks/exhaustive-deps
-  return useMemo(() => f(room.currentState), [room, f, numUpdates]);
-};
+}

src/useEvents.test.tsx

@@ -0,0 +1,88 @@
+/*
+Copyright 2025 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE in the repository root for full details.
+*/
+
+import { test } from "vitest";
+import { render, screen } from "@testing-library/react";
+import { type FC, useEffect, useState } from "react";
+import userEvent from "@testing-library/user-event";
+import { TypedEventEmitter } from "matrix-js-sdk";
+
+import { useTypedEventEmitterState } from "./useEvents";
+
+class TestEmitter extends TypedEventEmitter<"change", { change: () => void }> {
+  private state = 1;
+  public readonly getState = (): number => this.state;
+  public readonly getNegativeState = (): number => -this.state;
+  public readonly setState = (value: number): void => {
+    this.state = value;
+    this.emit("change");
+  };
+}
+
+test("useTypedEventEmitterState reacts to events", async () => {
+  const user = userEvent.setup();
+  const emitter = new TestEmitter();
+
+  const Test: FC = () => {
+    const value = useTypedEventEmitterState(
+      emitter,
+      "change",
+      emitter.getState,
+    );
+    return (
+      <>
+        <button onClick={() => emitter.setState(2)}>Change value</button>
+        <div>Value is {value}</div>
+      </>
+    );
+  };
+
+  render(<Test />);
+  screen.getByText("Value is 1");
+  await user.click(screen.getByText("Change value"));
+  screen.getByText("Value is 2");
+});
+
+test("useTypedEventEmitterState reacts to changes made by an effect mounted on the same render", () => {
+  const emitter = new TestEmitter();
+
+  const Test: FC = () => {
+    useEffect(() => emitter.setState(2), []);
+    const value = useTypedEventEmitterState(
+      emitter,
+      "change",
+      emitter.getState,
+    );
+    return `Value is ${value}`;
+  };
+
+  render(<Test />);
+  screen.getByText("Value is 2");
+});
+
+test("useTypedEventEmitterState reacts to changes in getState", async () => {
+  const user = userEvent.setup();
+  const emitter = new TestEmitter();
+
+  const Test: FC = () => {
+    const [fn, setFn] = useState(() => emitter.getState);
+    const value = useTypedEventEmitterState(emitter, "change", fn);
+    return (
+      <>
+        <button onClick={() => setFn(() => emitter.getNegativeState)}>
+          Change getState
+        </button>
+        <div>Value is {value}</div>
+      </>
+    );
+  };
+
+  render(<Test />);
+  screen.getByText("Value is 1");
+  await user.click(screen.getByText("Change getState"));
+  screen.getByText("Value is -1");
+});

src/useEvents.ts

@@ -5,15 +5,17 @@ SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
 Please see LICENSE in the repository root for full details.
 */
 
-import { useEffect } from "react";
+import { useCallback, useEffect, useSyncExternalStore } from "react";
 
 import type {
   Listener,
   ListenerMap,
   TypedEventEmitter,
 } from "matrix-js-sdk/lib/models/typed-event-emitter";
 
-// Shortcut for registering a listener on an EventTarget
+/**
+ * Shortcut for registering a listener on an EventTarget.
+ */
 export function useEventTarget<T extends Event>(
   target: EventTarget | null | undefined,
   eventType: string,
@@ -33,7 +35,9 @@ export function useEventTarget<T extends Event>(
   }, [target, eventType, listener, options]);
 }
 
-// Shortcut for registering a listener on a TypedEventEmitter
+/**
+ * Shortcut for registering a listener on a TypedEventEmitter.
+ */
 export function useTypedEventEmitter<
   Events extends string,
   Arguments extends ListenerMap<Events>,
@@ -50,3 +54,33 @@ export function useTypedEventEmitter<
     };
   }, [emitter, eventType, listener]);
 }
+
+/**
+ * Reactively tracks a value which is recalculated whenever the provided event
+ * emitter emits an event. This is useful for bridging state from matrix-js-sdk
+ * into React.
+ */
+export function useTypedEventEmitterState<
+  Events extends string,
+  Arguments extends ListenerMap<Events>,
+  T extends Events,
+  State,
+>(
+  emitter: TypedEventEmitter<Events, Arguments>,
+  eventType: T,
+  getState: () => State,
+): State {
+  const subscribe = useCallback(
+    (onChange: () => void) => {
+      emitter.on(eventType, onChange as Listener<Events, Arguments, T>);
+      return (): void => {
+        emitter.off(eventType, onChange as Listener<Events, Arguments, T>);
+      };
+    },
+    [emitter, eventType],
+  );
+  // See the React docs for useSyncExternalStore; given that we're trying to
+  // bridge state from an external source into React, using this hook is exactly
+  // what React recommends.
+  return useSyncExternalStore(subscribe, getState);
+}

src/useLocalStorage.test.tsx

@@ -0,0 +1,48 @@
+/*
+Copyright 2025 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE in the repository root for full details.
+*/
+
+import { test } from "vitest";
+import { render, screen } from "@testing-library/react";
+import { type FC, useEffect, useState } from "react";
+import userEvent from "@testing-library/user-event";
+
+import { setLocalStorageItem, useLocalStorage } from "./useLocalStorage";
+
+test("useLocalStorage reacts to changes made by an effect mounted on the same render", () => {
+  localStorage.clear();
+  const Test: FC = () => {
+    useEffect(() => setLocalStorageItem("my-value", "Hello!"), []);
+    const [myValue] = useLocalStorage("my-value");
+    return myValue;
+  };
+  render(<Test />);
+  screen.getByText("Hello!");
+});
+
+test("useLocalStorage reacts to key changes", async () => {
+  localStorage.clear();
+  localStorage.setItem("value-1", "1");
+  localStorage.setItem("value-2", "2");
+
+  const Test: FC = () => {
+    const [key, setKey] = useState("value-1");
+    const [value] = useLocalStorage(key);
+    if (key !== `value-${value}`) throw new Error("Value is out of sync");
+    return (
+      <>
+        <button onClick={() => setKey("value-2")}>Switch keys</button>
+        <div>Value is: {value}</div>
+      </>
+    );
+  };
+  const user = userEvent.setup();
+  render(<Test />);
+
+  screen.getByText("Value is: 1");
+  await user.click(screen.getByRole("button", { name: "Switch keys" }));
+  screen.getByText("Value is: 2");
+});