From f3e08de7fcfa66bf347a3f228ca6c454d1813595 Mon Sep 17 00:00:00 2001 From: Germain Date: Wed, 30 Aug 2023 11:42:00 +0100 Subject: [PATCH 1/5] Update member count on room summary update --- src/hooks/useRoomMembers.ts | 40 ++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/src/hooks/useRoomMembers.ts b/src/hooks/useRoomMembers.ts index b9db87bc2de..ca396580dfd 100644 --- a/src/hooks/useRoomMembers.ts +++ b/src/hooks/useRoomMembers.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { useState } from "react"; +import { useCallback, useState } from "react"; import { Room, RoomEvent, RoomMember, RoomStateEvent } from "matrix-js-sdk/src/matrix"; import { throttle } from "lodash"; @@ -59,26 +59,30 @@ export const useRoomMemberCount = ( const { throttleWait, includeFunctional } = opts; + const onMembershipUpdate = useCallback(() => { + // At the time where `RoomStateEvent.Members` is emitted the + // summary API has not had a chance to update the `summaryJoinedMemberCount` + // value, therefore handling the logic locally here. + // + // Tracked as part of https://github.com/vector-im/element-web/issues/26033 + const membersCount = includeFunctional + ? room.getMembers().reduce((count, m) => { + return m.membership === "join" ? count + 1 : count; + }, 0) + : getJoinedNonFunctionalMembers(room).length; + setCount(membersCount); + }, [includeFunctional, room]); + useTypedEventEmitter( room.currentState, RoomStateEvent.Members, - throttle( - () => { - // At the time where `RoomStateEvent.Members` is emitted the - // summary API has not had a chance to update the `summaryJoinedMemberCount` - // value, therefore handling the logic locally here. - // - // Tracked as part of https://github.com/vector-im/element-web/issues/26033 - const membersCount = includeFunctional - ? room.getMembers().reduce((count, m) => { - return m.membership === "join" ? count + 1 : count; - }, 0) - : getJoinedNonFunctionalMembers(room).length; - setCount(membersCount); - }, - throttleWait, - { leading: true, trailing: true }, - ), + throttle(onMembershipUpdate, throttleWait, { leading: true, trailing: true }), + ); + + useTypedEventEmitter( + room, + RoomEvent.Summary, + throttle(onMembershipUpdate, throttleWait, { leading: true, trailing: true }), ); return count; }; From 93712fdb4b1bcb2cc0cacb2742aaa9e6569faed7 Mon Sep 17 00:00:00 2001 From: Germain Date: Wed, 30 Aug 2023 11:46:28 +0100 Subject: [PATCH 2/5] Fix performance regression for large rooms --- src/hooks/room/useRoomCallStatus.ts | 2 +- src/hooks/useRoomMembers.ts | 27 ++++----------------------- 2 files changed, 5 insertions(+), 24 deletions(-) diff --git a/src/hooks/room/useRoomCallStatus.ts b/src/hooks/room/useRoomCallStatus.ts index 5de205ba768..0beb4429f64 100644 --- a/src/hooks/room/useRoomCallStatus.ts +++ b/src/hooks/room/useRoomCallStatus.ts @@ -67,7 +67,7 @@ export const useRoomCallStatus = ( const hasGroupCall = useCall(room.roomId) !== null; - const memberCount = useRoomMemberCount(room, { includeFunctional: false }); + const memberCount = useRoomMemberCount(room); const [mayEditWidgets, mayCreateElementCalls] = useTypedEventEmitterState( room, diff --git a/src/hooks/useRoomMembers.ts b/src/hooks/useRoomMembers.ts index ca396580dfd..51cf0eef8f5 100644 --- a/src/hooks/useRoomMembers.ts +++ b/src/hooks/useRoomMembers.ts @@ -19,7 +19,6 @@ import { Room, RoomEvent, RoomMember, RoomStateEvent } from "matrix-js-sdk/src/m import { throttle } from "lodash"; import { useTypedEventEmitter } from "./useEventEmitter"; -import { getJoinedNonFunctionalMembers } from "../utils/room/getJoinedNonFunctionalMembers"; // Hook to simplify watching Matrix Room joined members export const useRoomMembers = (room: Room, throttleWait = 250): RoomMember[] => { @@ -43,35 +42,17 @@ type RoomMemberCountOpts = { * Wait time between room member count update */ throttleWait?: number; - /** - * Whether to include functional members (bots, etc...) in the room count - * @default true - */ - includeFunctional: boolean; }; // Hook to simplify watching Matrix Room joined member count -export const useRoomMemberCount = ( - room: Room, - opts: RoomMemberCountOpts = { throttleWait: 250, includeFunctional: true }, -): number => { +export const useRoomMemberCount = (room: Room, opts: RoomMemberCountOpts = { throttleWait: 250 }): number => { const [count, setCount] = useState(room.getJoinedMemberCount()); - const { throttleWait, includeFunctional } = opts; + const { throttleWait } = opts; const onMembershipUpdate = useCallback(() => { - // At the time where `RoomStateEvent.Members` is emitted the - // summary API has not had a chance to update the `summaryJoinedMemberCount` - // value, therefore handling the logic locally here. - // - // Tracked as part of https://github.com/vector-im/element-web/issues/26033 - const membersCount = includeFunctional - ? room.getMembers().reduce((count, m) => { - return m.membership === "join" ? count + 1 : count; - }, 0) - : getJoinedNonFunctionalMembers(room).length; - setCount(membersCount); - }, [includeFunctional, room]); + setCount(room.getJoinedMemberCount()); + }, [room]); useTypedEventEmitter( room.currentState, From 922d77d6e5b5c4bfacbc9dafd9a329bf6d48983b Mon Sep 17 00:00:00 2001 From: Germain Date: Thu, 31 Aug 2023 09:17:03 +0100 Subject: [PATCH 3/5] Update useRoomMemberCount JSDoc --- src/hooks/useRoomMembers.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/hooks/useRoomMembers.ts b/src/hooks/useRoomMembers.ts index 51cf0eef8f5..f92831db752 100644 --- a/src/hooks/useRoomMembers.ts +++ b/src/hooks/useRoomMembers.ts @@ -44,7 +44,12 @@ type RoomMemberCountOpts = { throttleWait?: number; }; -// Hook to simplify watching Matrix Room joined member count +/** + * Returns a count of members in a given room + * @param room the room to track. + * @param opts The options. + * @returns the room member count. + */ export const useRoomMemberCount = (room: Room, opts: RoomMemberCountOpts = { throttleWait: 250 }): number => { const [count, setCount] = useState(room.getJoinedMemberCount()); @@ -60,6 +65,10 @@ export const useRoomMemberCount = (room: Room, opts: RoomMemberCountOpts = { thr throttle(onMembershipUpdate, throttleWait, { leading: true, trailing: true }), ); + /** + * `room.getJoinedMemberCount()` caches the member count behind the room summary + * So we need to re-compute the member count when the summary gets updated + */ useTypedEventEmitter( room, RoomEvent.Summary, From ab420cab8c78120f2d63c503bff41aec777b78c0 Mon Sep 17 00:00:00 2001 From: Germain Date: Thu, 31 Aug 2023 09:35:03 +0100 Subject: [PATCH 4/5] Improve throttle updates definition --- src/hooks/useRoomMembers.ts | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/hooks/useRoomMembers.ts b/src/hooks/useRoomMembers.ts index f92831db752..811962eb4d8 100644 --- a/src/hooks/useRoomMembers.ts +++ b/src/hooks/useRoomMembers.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { useCallback, useState } from "react"; +import { useCallback, useMemo, useState } from "react"; import { Room, RoomEvent, RoomMember, RoomStateEvent } from "matrix-js-sdk/src/matrix"; import { throttle } from "lodash"; @@ -55,25 +55,29 @@ export const useRoomMemberCount = (room: Room, opts: RoomMemberCountOpts = { thr const { throttleWait } = opts; - const onMembershipUpdate = useCallback(() => { + const updateMembershipCount = useCallback((): void => { setCount(room.getJoinedMemberCount()); }, [room]); - useTypedEventEmitter( - room.currentState, - RoomStateEvent.Members, - throttle(onMembershipUpdate, throttleWait, { leading: true, trailing: true }), + const throttledUpdate = useMemo( + () => + throttle( + () => { + updateMembershipCount(); + }, + throttleWait, + { leading: true, trailing: true }, + ), + [throttleWait, updateMembershipCount], ); + useTypedEventEmitter(room.currentState, RoomStateEvent.Members, throttledUpdate); + /** * `room.getJoinedMemberCount()` caches the member count behind the room summary * So we need to re-compute the member count when the summary gets updated */ - useTypedEventEmitter( - room, - RoomEvent.Summary, - throttle(onMembershipUpdate, throttleWait, { leading: true, trailing: true }), - ); + useTypedEventEmitter(room, RoomEvent.Summary, throttledUpdate); return count; }; From f37b29fbf179b065bfdc0c6ec95c3d35931354d1 Mon Sep 17 00:00:00 2001 From: Germain Date: Thu, 31 Aug 2023 10:43:23 +0100 Subject: [PATCH 5/5] remove useCallback --- src/hooks/useRoomMembers.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/hooks/useRoomMembers.ts b/src/hooks/useRoomMembers.ts index 811962eb4d8..f054d7337b0 100644 --- a/src/hooks/useRoomMembers.ts +++ b/src/hooks/useRoomMembers.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { useCallback, useMemo, useState } from "react"; +import { useMemo, useState } from "react"; import { Room, RoomEvent, RoomMember, RoomStateEvent } from "matrix-js-sdk/src/matrix"; import { throttle } from "lodash"; @@ -55,20 +55,16 @@ export const useRoomMemberCount = (room: Room, opts: RoomMemberCountOpts = { thr const { throttleWait } = opts; - const updateMembershipCount = useCallback((): void => { - setCount(room.getJoinedMemberCount()); - }, [room]); - const throttledUpdate = useMemo( () => throttle( () => { - updateMembershipCount(); + setCount(room.getJoinedMemberCount()); }, throttleWait, { leading: true, trailing: true }, ), - [throttleWait, updateMembershipCount], + [room, throttleWait], ); useTypedEventEmitter(room.currentState, RoomStateEvent.Members, throttledUpdate);