diff --git a/src/app/components/read-receipt-avatars/ReadReceiptAvatars.tsx b/src/app/components/read-receipt-avatars/ReadReceiptAvatars.tsx index 2c217fa08..6b3d12597 100644 --- a/src/app/components/read-receipt-avatars/ReadReceiptAvatars.tsx +++ b/src/app/components/read-receipt-avatars/ReadReceiptAvatars.tsx @@ -1,5 +1,5 @@ -import React, { useEffect, useState } from 'react'; -import { Room, RoomStateEvent, RoomStateEventHandlerMap } from 'matrix-js-sdk'; +import React, { useState } from 'react'; +import { Room } from 'matrix-js-sdk'; import { Icon, Icons, @@ -12,7 +12,6 @@ import { config, } from 'folds'; import FocusTrap from 'focus-trap-react'; -import { useMatrixClient } from '../../hooks/useMatrixClient'; import { useSetting } from '../../state/hooks/settings'; import { settingsAtom } from '../../state/settings'; import { getMemberName } from '../../utils/room'; @@ -21,8 +20,8 @@ import { StackedAvatar } from '../stacked-avatar'; import { EventReaders } from '../event-readers'; import { stopPropagation } from '../../utils/keyboard'; import { useModalStyle } from '../../hooks/useModalStyle'; -import { useForceUpdate } from '../../hooks/useForceUpdate'; import { useMemberAvatar } from '../../hooks/useMemberAvatar'; +import { useRoomMembersChange } from '../../hooks/useRoomMemberChange'; import * as css from './ReadReceiptAvatars.css'; const MAX_DISPLAY = 5; @@ -50,32 +49,15 @@ export function ReadReceiptAvatars({ eventId: string; userIds: string[]; }) { - const mx = useMatrixClient(); const [open, setOpen] = useState(false); const [lotusTerminal] = useSetting(settingsAtom, 'lotusTerminal'); const modalStyle = useModalStyle(360); - const [, forceUpdate] = useForceUpdate(); - // The avatars + names below are read from room member state at render time, so - // they only refresh when this component re-renders (which the timeline does on a - // receipt change). Re-render on a member-state change of any displayed reader so - // an avatar/display-name update shows live. RoomStateEvent.Members fires for name, - // avatar AND membership changes (RoomMemberEvent has no Avatar signal). - useEffect(() => { - const handleMembers: RoomStateEventHandlerMap[RoomStateEvent.Members] = ( - event, - _state, - member, - ) => { - if (event.getRoomId() === room.roomId && userIds.includes(member.userId)) { - forceUpdate(); - } - }; - mx.on(RoomStateEvent.Members, handleMembers); - return () => { - mx.removeListener(RoomStateEvent.Members, handleMembers); - }; - }, [mx, room.roomId, userIds, forceUpdate]); + // The tooltip names below are read from room member state at render time. + // Re-render on a member-state change of any displayed reader so a display-name + // update shows live (each avatar handles its own via useMemberAvatar). Uses the + // shared member-change store — one global listener for the whole app (PERF-3). + useRoomMembersChange(room.roomId, userIds); if (userIds.length === 0) return null; diff --git a/src/app/hooks/useMemberAvatar.ts b/src/app/hooks/useMemberAvatar.ts index e13da6087..d089e23e6 100644 --- a/src/app/hooks/useMemberAvatar.ts +++ b/src/app/hooks/useMemberAvatar.ts @@ -1,8 +1,7 @@ -import { useEffect } from 'react'; -import { Room, RoomStateEvent, RoomStateEventHandlerMap } from 'matrix-js-sdk'; +import { Room } from 'matrix-js-sdk'; import { useMatrixClient } from './useMatrixClient'; import { useMediaAuthentication } from './useMediaAuthentication'; -import { useForceUpdate } from './useForceUpdate'; +import { useRoomMemberChange } from './useRoomMemberChange'; import { getMemberName } from '../utils/room'; import { mxcUrlToHttp } from '../utils/matrix'; @@ -15,9 +14,8 @@ export type MemberAvatar = { * Resolve a room member's display name and avatar http url, staying reactive to * that member's profile (name/avatar/membership) changes. * - * Mirrors the reactivity pattern in ReadReceiptAvatars (N6): subscribe to - * `RoomStateEvent.Members` filtered to this room + userId and force a re-render, - * since `RoomMemberEvent` has no dedicated Avatar signal. + * Stays reactive to this member's profile (name/avatar/membership) changes via + * the shared member-change store (one global listener for the whole app). */ export const useMemberAvatar = ( room: Room, @@ -28,23 +26,8 @@ export const useMemberAvatar = ( ): MemberAvatar => { const mx = useMatrixClient(); const useAuthentication = useMediaAuthentication(); - const [, forceUpdate] = useForceUpdate(); - useEffect(() => { - const handleMembers: RoomStateEventHandlerMap[RoomStateEvent.Members] = ( - event, - _state, - member, - ) => { - if (event.getRoomId() === room.roomId && member.userId === userId) { - forceUpdate(); - } - }; - mx.on(RoomStateEvent.Members, handleMembers); - return () => { - mx.removeListener(RoomStateEvent.Members, handleMembers); - }; - }, [mx, room.roomId, userId, forceUpdate]); + useRoomMemberChange(room.roomId, userId); const name = getMemberName(room, userId); const avatarMxc = room.getMember(userId)?.getMxcAvatarUrl(); diff --git a/src/app/hooks/useRoomMemberChange.test.ts b/src/app/hooks/useRoomMemberChange.test.ts new file mode 100644 index 000000000..9e14f0a61 --- /dev/null +++ b/src/app/hooks/useRoomMemberChange.test.ts @@ -0,0 +1,67 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import type { MatrixClient } from 'matrix-js-sdk'; +import { memberChangeStore } from './useRoomMemberChange'; + +// Minimal fake client: records registered handlers and can emit a member change. +// The store registers exactly one RoomStateEvent.Members handler; we invoke it +// with the same (event, state, member) shape the SDK uses. +const makeFakeMx = () => { + const handlers: Array<(e: unknown, s: unknown, m: unknown) => void> = []; + const mx = { + on: (_event: unknown, h: (e: unknown, s: unknown, m: unknown) => void) => { + handlers.push(h); + }, + removeListener: (_event: unknown, h: (e: unknown, s: unknown, m: unknown) => void) => { + const i = handlers.indexOf(h); + if (i >= 0) handlers.splice(i, 1); + }, + }; + return { + mx: mx as unknown as MatrixClient, + listenerCount: () => handlers.length, + emit: (roomId: string, userId: string) => + handlers.forEach((h) => h({ getRoomId: () => roomId }, undefined, { userId })), + }; +}; + +test('fans out only to the matching roomId|userId', () => { + const { mx, emit } = makeFakeMx(); + let a = 0; + let b = 0; + memberChangeStore.subscribe(mx, '!fanout', '@a', () => { + a += 1; + }); + memberChangeStore.subscribe(mx, '!fanout', '@b', () => { + b += 1; + }); + + emit('!fanout', '@a'); + assert.equal(a, 1); + assert.equal(b, 0); + + emit('!other', '@a'); // different room, same user -> no fire + assert.equal(a, 1); +}); + +test('registers exactly one client listener regardless of subscriber count', () => { + const { mx, listenerCount } = makeFakeMx(); + memberChangeStore.subscribe(mx, '!one', '@a', () => {}); + memberChangeStore.subscribe(mx, '!one', '@b', () => {}); + memberChangeStore.subscribe(mx, '!one2', '@c', () => {}); + assert.equal(listenerCount(), 1); +}); + +test('idempotent unsubscribe does not evict a re-subscribed key', () => { + const { mx, emit } = makeFakeMx(); + const unsubA = memberChangeStore.subscribe(mx, '!idem', '@a', () => {}); + unsubA(); + + let count = 0; + memberChangeStore.subscribe(mx, '!idem', '@a', () => { + count += 1; + }); + unsubA(); // double-invoke of the first unsub must not drop the new subscriber + emit('!idem', '@a'); + assert.equal(count, 1); +}); diff --git a/src/app/hooks/useRoomMemberChange.ts b/src/app/hooks/useRoomMemberChange.ts new file mode 100644 index 000000000..f79da5e00 --- /dev/null +++ b/src/app/hooks/useRoomMemberChange.ts @@ -0,0 +1,102 @@ +import { useEffect } from 'react'; +import { MatrixClient, RoomStateEvent, RoomStateEventHandlerMap } from 'matrix-js-sdk'; +import { useMatrixClient } from './useMatrixClient'; +import { useForceUpdate } from './useForceUpdate'; + +type MemberListener = () => void; + +const memberKey = (roomId: string, userId: string): string => `${roomId}|${userId}`; + +/** + * Shared room-member-change store. Previously every ReadReceiptAvatars row and + * every useMemberAvatar registered its own global `RoomStateEvent.Members` + * listener — ~6 per receipt row — each firing on ANY membership / display-name / + * avatar change in ANY room (PERF-3). This registers exactly ONE listener total + * and fans out to subscribers keyed by `roomId|userId`. + * + * (`RoomStateEvent.Members` is used rather than `RoomMemberEvent` because the + * latter has no dedicated avatar signal.) + */ +class MemberChangeStore { + private mx: MatrixClient | undefined; + + private started = false; + + private subscribers = new Map>(); + + private handleMembers: RoomStateEventHandlerMap[RoomStateEvent.Members] = ( + event, + _state, + member, + ) => { + const roomId = event.getRoomId(); + if (!roomId) return; + const subs = this.subscribers.get(memberKey(roomId, member.userId)); + if (!subs) return; + subs.forEach((cb) => cb()); + }; + + private start(mx: MatrixClient): void { + if (this.started && this.mx === mx) return; + if (this.started && this.mx) { + this.mx.removeListener(RoomStateEvent.Members, this.handleMembers); + } + this.mx = mx; + mx.on(RoomStateEvent.Members, this.handleMembers); + this.started = true; + } + + subscribe(mx: MatrixClient, roomId: string, userId: string, cb: MemberListener): () => void { + this.start(mx); + const key = memberKey(roomId, userId); + let subs = this.subscribers.get(key); + if (!subs) { + subs = new Set(); + this.subscribers.set(key, subs); + } + subs.add(cb); + return () => { + subs.delete(cb); + // Only drop the registry entry if this is still the live set for the key, + // keeping unsubscribe idempotent against a double-invoke / re-subscribe. + if (subs.size === 0 && this.subscribers.get(key) === subs) { + this.subscribers.delete(key); + } + }; + } +} + +export const memberChangeStore = new MemberChangeStore(); + +/** + * Re-render the caller whenever the given room member's state (display name, + * avatar, or membership) changes, via the shared single-listener store. + */ +export const useRoomMemberChange = (roomId: string, userId: string): void => { + const mx = useMatrixClient(); + const [, forceUpdate] = useForceUpdate(); + useEffect( + () => memberChangeStore.subscribe(mx, roomId, userId, forceUpdate), + [mx, roomId, userId, forceUpdate], + ); +}; + +/** + * Multi-user variant: re-render when ANY of `userIds` changes. Subscribes to + * each id through the shared store from a single effect (no per-id hook). + */ +export const useRoomMembersChange = (roomId: string, userIds: string[]): void => { + const mx = useMatrixClient(); + const [, forceUpdate] = useForceUpdate(); + // Stable, order-independent dependency for the set of ids (the array identity + // is unstable, and re-ordering the same members shouldn't churn subscriptions). + const idsKey = [...userIds].sort().join(','); + useEffect(() => { + const unsubs = userIds.map((userId) => + memberChangeStore.subscribe(mx, roomId, userId, forceUpdate), + ); + return () => unsubs.forEach((unsub) => unsub()); + // `userIds` is captured via the stable `idsKey`; re-subscribes when it changes. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [mx, roomId, idsKey, forceUpdate]); +};