Internalize notification state handling for lists

This reduces the update cost of rooms changing, and fixes a bug where when a sublist became filtered it would change the notification count of the sublist.

This does change the expected usage of the state store to ensuring that only one place updates the rooms on the list states, which is currently the room list store. Ideally the state store could listen to the room list store to update itself, however due to a complicated require() loop it is not possible.

src/components/views/rooms/RoomSublist.tsx

@@ -38,7 +38,6 @@ import { DefaultTagID, TagID } from "../../../stores/room-list/models";
 import dis from "../../../dispatcher/dispatcher";
 import defaultDispatcher from "../../../dispatcher/dispatcher";
 import NotificationBadge from "./NotificationBadge";
-import { ListNotificationState } from "../../../stores/notifications/ListNotificationState";
 import AccessibleTooltipButton from "../elements/AccessibleTooltipButton";
 import { Key } from "../../../Keyboard";
 import { ActionPayload } from "../../../dispatcher/payloads";
@@ -50,6 +49,7 @@ import RoomListLayoutStore from "../../../stores/room-list/RoomListLayoutStore";
 import { arrayHasOrderChange } from "../../../utils/arrays";
 import { objectExcluding, objectHasValueChange } from "../../../utils/objects";
 import TemporaryTile from "./TemporaryTile";
+import { NotificationState } from "../../../stores/notifications/NotificationState";
 
 const SHOW_N_BUTTON_HEIGHT = 28; // As defined by CSS
 const RESIZE_HANDLE_HEIGHT = 4; // As defined by CSS
@@ -86,7 +86,6 @@ interface ResizeDelta {
 type PartialDOMRect = Pick<DOMRect, "left" | "top" | "height">;
 
 interface IState {
-    notificationState: ListNotificationState;
     contextMenuPosition: PartialDOMRect;
     isResizing: boolean;
     isExpanded: boolean; // used for the for expand of the sublist when the room list is being filtered
@@ -102,6 +101,7 @@ export default class RoomSublist extends React.Component<IProps, IState> {
     private layout: ListLayout;
     private heightAtStart: number;
     private isBeingFiltered: boolean;
+    private notificationState: NotificationState;
 
     constructor(props: IProps) {
         super(props);
@@ -109,8 +109,8 @@ export default class RoomSublist extends React.Component<IProps, IState> {
         this.layout = RoomListLayoutStore.instance.getLayoutFor(this.props.tagId);
         this.heightAtStart = 0;
         this.isBeingFiltered = !!RoomListStore.instance.getFirstNameFilterCondition();
+        this.notificationState = RoomNotificationStateStore.instance.getListState(this.props.tagId);
         this.state = {
-            notificationState: RoomNotificationStateStore.instance.getListState(this.props.tagId),
             contextMenuPosition: null,
             isResizing: false,
             isExpanded: this.isBeingFiltered ? this.isBeingFiltered : !this.layout.isCollapsed,
@@ -119,7 +119,6 @@ export default class RoomSublist extends React.Component<IProps, IState> {
         };
         // Why Object.assign() and not this.state.height? Because TypeScript says no.
         this.state = Object.assign(this.state, {height: this.calculateInitialHeight()});
-        this.state.notificationState.setRooms(this.state.rooms);
         this.dispatcherRef = defaultDispatcher.register(this.onAction);
         RoomListStore.instance.on(LISTS_UPDATE_EVENT, this.onListsUpdated);
     }
@@ -173,7 +172,6 @@ export default class RoomSublist extends React.Component<IProps, IState> {
     }
 
     public componentDidUpdate(prevProps: Readonly<IProps>, prevState: Readonly<IState>) {
-        this.state.notificationState.setRooms(this.state.rooms);
         const prevExtraTiles = prevState.filteredExtraTiles || prevProps.extraBadTilesThatShouldntExist;
         // as the rooms can come in one by one we need to reevaluate
         // the amount of available rooms to cap the amount of requested visible rooms by the layout
@@ -239,7 +237,6 @@ export default class RoomSublist extends React.Component<IProps, IState> {
     }
 
     public componentWillUnmount() {
-        this.state.notificationState.destroy();
         defaultDispatcher.unregister(this.dispatcherRef);
         RoomListStore.instance.off(LISTS_UPDATE_EVENT, this.onListsUpdated);
     }
@@ -409,8 +406,8 @@ export default class RoomSublist extends React.Component<IProps, IState> {
         } else {
             // find the first room with a count of the same colour as the badge count
             room = this.state.rooms.find((r: Room) => {
-                const notifState = this.state.notificationState.getForRoom(r);
+                const notifState = this.notificationState.getForRoom(r);
-                return notifState.count > 0 && notifState.color === this.state.notificationState.color;
+                return notifState.count > 0 && notifState.color === this.notificationState.color;
             });
         }
 
@@ -626,7 +623,7 @@ export default class RoomSublist extends React.Component<IProps, IState> {
                     const badge = (
                         <NotificationBadge
                             forceCount={true}
-                            notification={this.state.notificationState}
+                            notification={this.notificationState}
                             onClick={this.onBadgeClick}
                             tabIndex={tabIndex}
                             aria-label={ariaLabel}

src/stores/notifications/RoomNotificationStateStore.ts

@@ -29,6 +29,7 @@ export class RoomNotificationStateStore extends AsyncStoreWithClient<IState> {
     private static internalInstance = new RoomNotificationStateStore();
 
     private roomMap = new Map<Room, RoomNotificationState>();
+    private listMap = new Map<TagID, ListNotificationState>();
 
     private constructor() {
         super(defaultDispatcher, {});
@@ -52,21 +53,23 @@ export class RoomNotificationStateStore extends AsyncStoreWithClient<IState> {
     }
 
     /**
-     * Creates a new list notification state. The consumer is expected to set the rooms
+     * Gets an instance of the list state class for the given tag.
-     * on the notification state, and destroy the state when it no longer needs it.
+     * @param tagId The tag to get the notification state for.
-     * @param tagId The tag to create the notification state for.
      * @returns The notification state for the tag.
      */
     public getListState(tagId: TagID): ListNotificationState {
-        // Note: we don't cache these notification states as the consumer is expected to call
+        if (this.listMap.has(tagId)) {
-        // .setRooms() on the returned object, which could confuse other consumers.
+            return this.listMap.get(tagId);
+        }
 
         // TODO: Update if/when invites move out of the room list.
         const useTileCount = tagId === DefaultTagID.Invite;
         const getRoomFn: FetchRoomFn = (room: Room) => {
             return this.getRoomState(room);
         };
-        return new ListNotificationState(useTileCount, tagId, getRoomFn);
+        const state = new ListNotificationState(useTileCount, tagId, getRoomFn);
+        this.listMap.set(tagId, state);
+        return state;
     }
 
     /**

src/stores/room-list/RoomListStore.ts

@@ -33,6 +33,7 @@ import RoomListLayoutStore from "./RoomListLayoutStore";
 import { MarkedExecution } from "../../utils/MarkedExecution";
 import { AsyncStoreWithClient } from "../AsyncStoreWithClient";
 import { NameFilterCondition } from "./filters/NameFilterCondition";
+import { RoomNotificationStateStore } from "../notifications/RoomNotificationStateStore";
 
 interface IState {
     tagsEnabled?: boolean;
@@ -55,7 +56,12 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
     private algorithm = new Algorithm();
     private filterConditions: IFilterCondition[] = [];
     private tagWatcher = new TagWatcher(this);
-    private updateFn = new MarkedExecution(() => this.emit(LISTS_UPDATE_EVENT));
+    private updateFn = new MarkedExecution(() => {
+        for (const tagId of Object.keys(this.unfilteredLists)) {
+            RoomNotificationStateStore.instance.getListState(tagId).setRooms(this.unfilteredLists[tagId]);
+        }
+        this.emit(LISTS_UPDATE_EVENT);
+    });
 
     private readonly watchedSettings = [
         'feature_custom_tags',
@@ -72,6 +78,11 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> {
         this.algorithm.on(FILTER_CHANGED, this.onAlgorithmFilterUpdated);
     }
 
+    public get unfilteredLists(): ITagMap {
+        if (!this.algorithm) return {}; // No tags yet.
+        return this.algorithm.getUnfilteredRooms();
+    }
+
     public get orderedLists(): ITagMap {
         if (!this.algorithm) return {}; // No tags yet.
         return this.algorithm.getOrderedRooms();

src/stores/room-list/algorithms/Algorithm.ts

@@ -465,6 +465,10 @@ export class Algorithm extends EventEmitter {
         return this.filteredRooms;
     }
 
+    public getUnfilteredRooms(): ITagMap {
+        return this._cachedStickyRooms || this.cachedRooms;
+    }
+
     /**
      * This returns the same as getOrderedRooms(), but without the sticky room
      * map as it causes issues for sticky room handling (see sticky room handling