From 3cfba323f662f008d9243be60bbd9c8c54021e5a Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Mon, 2 Mar 2026 18:50:56 +0100 Subject: [PATCH] Room list: avoid excessive re-renders on room list store update or filter change (#32663) * perf(room list): clear room list item vm only when changing space Clearing all the item vms at every room list change is causing massive re-render of all the room list items. References to the vms are already removed when out of view (see RoomListViewMode.updateVisibleRooms) and handled by GC. * perf(room list): avoid to re-render filters at every room list update RoomListView re-renders on update but the filters (children) don't need to. Add a memo to avoid excessive-rerenders. * chore: add `keepIfSame` to do diff on vm objects * perf(room list): avoid to create new filter ids and keys when the room list is updated The filterKeys are passed in the virtuoso context so it should reduce internal computing The filter ids array has always the same value, there is no point to create a new instance. * test(room list): remove no longer relevant test * test(room list): add new test to ensure that room list item vms are preserved --- apps/web/src/utils/keepIfSame.ts | 20 ++++++++++++ .../room-list/RoomListViewViewModel.ts | 20 ++++++------ .../test/unit-tests/utils/keepIfSame-test.ts | 22 +++++++++++++ .../room-list/RoomListViewViewModel-test.tsx | 32 ++++++++----------- .../RoomListPrimaryFilters.tsx | 8 ++--- 5 files changed, 71 insertions(+), 31 deletions(-) create mode 100644 apps/web/src/utils/keepIfSame.ts create mode 100644 apps/web/test/unit-tests/utils/keepIfSame-test.ts diff --git a/apps/web/src/utils/keepIfSame.ts b/apps/web/src/utils/keepIfSame.ts new file mode 100644 index 0000000000..8346ae389c --- /dev/null +++ b/apps/web/src/utils/keepIfSame.ts @@ -0,0 +1,20 @@ +/* + * Copyright 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +import { isEqual } from "lodash"; + +/** + * Returns the current value if it is deeply equal to the next value, otherwise returns the next value. + * This is useful to prevent unnecessary re-renders in React components when the value has not changed. + * @param current The current value + * @param next The next value + * @returns The current value if it is deeply equal to the next value, otherwise the next value + */ +export function keepIfSame(current: T, next: T): T { + if (isEqual(current, next)) return current; + return next; +} diff --git a/apps/web/src/viewmodels/room-list/RoomListViewViewModel.ts b/apps/web/src/viewmodels/room-list/RoomListViewViewModel.ts index a3618b93af..ed1c305796 100644 --- a/apps/web/src/viewmodels/room-list/RoomListViewViewModel.ts +++ b/apps/web/src/viewmodels/room-list/RoomListViewViewModel.ts @@ -25,6 +25,7 @@ import { RoomNotificationStateStore } from "../../stores/notifications/RoomNotif import { RoomListItemViewModel } from "./RoomListItemViewModel"; import { SdkContextClass } from "../../contexts/SDKContext"; import { hasCreateRoomRights } from "./utils"; +import { keepIfSame } from "../../utils/keepIfSame"; interface RoomListViewViewModelProps { client: MatrixClient; @@ -133,9 +134,6 @@ export class RoomListViewViewModel // Update roomsMap immediately before clearing VMs this.updateRoomsMap(this.roomsResult); - // Clear view models since room list changed - this.clearViewModels(); - this.updateRoomListData(); }; @@ -291,11 +289,13 @@ export class RoomListViewViewModel const newSpaceId = this.roomsResult.spaceId; - // Clear view models since room list structure changed - this.clearViewModels(); - // Detect space change if (oldSpaceId !== newSpaceId) { + // Clear view models when the space changes + // We only want to do this on space changes, not on regular list updates, to preserve view models when possible + // The view models are disposed when scrolling out of view (handled by updateVisibleRooms) + this.clearViewModels(); + // Space changed - get the last selected room for the new space to prevent flicker const lastSelectedRoom = SpaceStore.instance.getLastSelectedRoomIdForSpace(newSpaceId); @@ -408,13 +408,16 @@ export class RoomListViewViewModel // Build the complete state atomically to ensure consistency // roomIds and roomListState must always be in sync const roomIds = this.roomIds; + + // Update filter keys - only update if they have actually changed to prevent unnecessary re-renders of the room list + const previousFilterKeys = this.snapshot.current.roomListState.filterKeys; + const newFilterKeys = this.roomsResult.filterKeys?.map((k) => String(k)); const roomListState: RoomListViewState = { activeRoomIndex, spaceId: this.roomsResult.spaceId, - filterKeys: this.roomsResult.filterKeys?.map((k) => String(k)), + filterKeys: keepIfSame(previousFilterKeys, newFilterKeys), }; - const filterIds = [...filterKeyToIdMap.values()]; const activeFilterId = this.activeFilter !== undefined ? filterKeyToIdMap.get(this.activeFilter) : undefined; const isRoomListEmpty = roomIds.length === 0; const isLoadingRooms = RoomListStoreV3.instance.isLoadingRooms; @@ -423,7 +426,6 @@ export class RoomListViewViewModel this.snapshot.merge({ isLoadingRooms, isRoomListEmpty, - filterIds, activeFilterId, roomListState, roomIds, diff --git a/apps/web/test/unit-tests/utils/keepIfSame-test.ts b/apps/web/test/unit-tests/utils/keepIfSame-test.ts new file mode 100644 index 0000000000..b4ea3393b5 --- /dev/null +++ b/apps/web/test/unit-tests/utils/keepIfSame-test.ts @@ -0,0 +1,22 @@ +/* + * Copyright 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial + * Please see LICENSE files in the repository root for full details. + */ + +import { keepIfSame } from "../../../src/utils/keepIfSame"; + +describe("keepIfSame", () => { + it("returns the next value if the current and next values are not deeply equal", () => { + const current = { a: 1 }; + const next = { a: 2 }; + expect(keepIfSame(current, next)).toBe(next); + }); + + it("returns the current value if the current and next values are deeply equal", () => { + const current = { a: 1 }; + const next = { a: 1 }; + expect(keepIfSame(current, next)).toBe(current); + }); +}); diff --git a/apps/web/test/viewmodels/room-list/RoomListViewViewModel-test.tsx b/apps/web/test/viewmodels/room-list/RoomListViewViewModel-test.tsx index c896d3111d..abdd6d10f7 100644 --- a/apps/web/test/viewmodels/room-list/RoomListViewViewModel-test.tsx +++ b/apps/web/test/viewmodels/room-list/RoomListViewViewModel-test.tsx @@ -124,6 +124,20 @@ describe("RoomListViewViewModel", () => { expect(viewModel.getSnapshot().isLoadingRooms).toBe(false); }); + + // This test ensures that the room list item vms are preserved when the room list is changing + it("should keep existing view model when ListsUpdate event fires", () => { + viewModel = new RoomListViewViewModel({ client: matrixClient }); + + // Create view model for room1 + const room1VM = viewModel.getRoomItemViewModel("!room1:server"); + expect(room1VM).toBeDefined(); + + RoomListStoreV3.instance.emit(RoomListStoreV3Event.ListsUpdate); + + // View model should be still valid + expect(room1VM.isDisposed).toBe(false); + }); }); describe("Space switching", () => { @@ -295,24 +309,6 @@ describe("RoomListViewViewModel", () => { expect(viewModel.getSnapshot().activeFilterId).toBeUndefined(); expect(viewModel.getSnapshot().roomIds).toEqual(["!room1:server", "!room2:server", "!room3:server"]); }); - - it("should clear view models when filter changes", () => { - viewModel = new RoomListViewViewModel({ client: matrixClient }); - - // Get view models - const vm1 = viewModel.getRoomItemViewModel("!room1:server"); - const disposeSpy = jest.spyOn(vm1, "dispose"); - - jest.spyOn(RoomListStoreV3.instance, "getSortedRoomsInActiveSpace").mockReturnValue({ - spaceId: "home", - rooms: [room2], - filterKeys: [FilterKey.UnreadFilter], - }); - - viewModel.onToggleFilter("unread"); - - expect(disposeSpy).toHaveBeenCalled(); - }); }); describe("Room item view models", () => { diff --git a/packages/shared-components/src/room-list/RoomListPrimaryFilters/RoomListPrimaryFilters.tsx b/packages/shared-components/src/room-list/RoomListPrimaryFilters/RoomListPrimaryFilters.tsx index 561544a3a5..bbc26692e3 100644 --- a/packages/shared-components/src/room-list/RoomListPrimaryFilters/RoomListPrimaryFilters.tsx +++ b/packages/shared-components/src/room-list/RoomListPrimaryFilters/RoomListPrimaryFilters.tsx @@ -5,7 +5,7 @@ * Please see LICENSE files in the repository root for full details. */ -import React, { type JSX, useId, useState } from "react"; +import React, { type JSX, memo, useId, useState } from "react"; import { ChatFilter, IconButton } from "@vector-im/compound-web"; import ChevronDownIcon from "@vector-im/compound-design-tokens/assets/web/icons/chevron-down"; @@ -53,11 +53,11 @@ export interface RoomListPrimaryFiltersProps { * The primary filters component for the room list. * Displays a collapsible list of filters with expand/collapse functionality. */ -export const RoomListPrimaryFilters: React.FC = ({ +export const RoomListPrimaryFilters = memo(function RoomListPrimaryFilters({ filterIds, activeFilterId, onToggleFilter, -}): JSX.Element | null => { +}: RoomListPrimaryFiltersProps): JSX.Element | null { const id = useId(); const [isExpanded, setIsExpanded] = useState(false); @@ -113,4 +113,4 @@ export const RoomListPrimaryFilters: React.FC = ({ ); -}; +});