perf(receipts): shared member-change store instead of per-row listeners (PERF-3)
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. Add a module-level MemberChangeStore (mirroring the PERF-1 presence store) that registers exactly ONE global Members listener and fans out to subscribers keyed by roomId|userId. Two hooks: useRoomMemberChange (single) and useRoomMembersChange (multi, one effect). useMemberAvatar and ReadReceiptAvatars use them; behavior (re-render triggers) is byte-for-byte equivalent. Unsubscribe is idempotent via a set-identity guard; the multi-hook key is order-independent. Unit-tested (key-scoped fan-out, single shared listener, idempotent unsubscribe). Reviewed by two passes (lifecycle/closure + behavioral equivalence) — clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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;
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
@@ -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<string, Set<MemberListener>>();
|
||||
|
||||
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]);
|
||||
};
|
||||
Reference in New Issue
Block a user