Step 8.5: Isolate RightPanelStore from RoomViewStore

src/dispatcher/actions.ts

@@ -259,4 +259,10 @@ export enum Action {
      * Fired when clicking user name from group view
      */
     ViewStartChatOrReuse = "view_start_chat_or_reuse",
+
+    /**
+     * Fired when the user's active room changed, possibly from/to a non-room view.
+     * Payload: ActiveRoomChangedPayload
+     */
+    ActiveRoomChanged = "active_room_changed",
 }

src/dispatcher/payloads/ActiveRoomChangedPayload.ts

@@ -0,0 +1,27 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { Optional } from "matrix-events-sdk";
+
+import { Action } from "../actions";
+import { ActionPayload } from "../payloads";
+
+export interface ActiveRoomChangedPayload extends ActionPayload {
+    action: Action.ActiveRoomChanged;
+
+    oldRoomId: Optional<string>;
+    newRoomId: Optional<string>;
+}

src/stores/ReadyWatchingStore.ts

@@ -60,7 +60,13 @@ export abstract class ReadyWatchingStore extends EventEmitter implements IDestro
         // Default implementation is to do nothing.
     }
 
+    protected onDispatcherAction(payload: ActionPayload) {
+        // Default implementation is to do nothing.
+    }
+
     private onAction = async (payload: ActionPayload) => {
+        this.onDispatcherAction(payload);
+
         if (payload.action === 'MatrixActions.sync') {
             // Only set the client on the transition into the PREPARED state.
             // Everything after this is unnecessary (we only need to know once we have a client)

src/stores/RoomViewStore.tsx

@@ -24,7 +24,7 @@ import { ViewRoom as ViewRoomEvent } from "matrix-analytics-events/types/typescr
 import { JoinedRoom as JoinedRoomEvent } from "matrix-analytics-events/types/typescript/JoinedRoom";
 import { JoinRule } from "matrix-js-sdk/src/@types/partials";
 import { Room } from "matrix-js-sdk/src/models/room";
-import { ClientEvent } from "matrix-js-sdk/src/client";
+import { ClientEvent, MatrixClient } from "matrix-js-sdk/src/client";
 
 import dis from '../dispatcher/dispatcher';
 import { MatrixClientPeg } from '../MatrixClientPeg';
@@ -46,6 +46,7 @@ import { JoinRoomErrorPayload } from "../dispatcher/payloads/JoinRoomErrorPayloa
 import { ViewRoomErrorPayload } from "../dispatcher/payloads/ViewRoomErrorPayload";
 import RoomSettingsDialog from "../components/views/dialogs/RoomSettingsDialog";
 import ErrorDialog from "../components/views/dialogs/ErrorDialog";
+import { ActiveRoomChangedPayload } from "../dispatcher/payloads/ActiveRoomChangedPayload";
 
 const NUM_JOIN_RETRY = 5;
 
@@ -93,6 +94,7 @@ export class RoomViewStore extends Store<ActionPayload> {
     public static readonly instance = new RoomViewStore();
 
     private state = INITIAL_STATE; // initialize state
+    private forcedMatrixClient: MatrixClient;
 
     // Keep these out of state to avoid causing excessive/recursive updates
     private roomIdActivityListeners: Record<string, Listener[]> = {};
@@ -101,6 +103,14 @@ export class RoomViewStore extends Store<ActionPayload> {
         super(dis);
     }
 
+    private get matrixClient(): MatrixClient {
+        return this.forcedMatrixClient || MatrixClientPeg.get();
+    }
+
+    public useUnitTestClient(client: MatrixClient) {
+        this.forcedMatrixClient = client;
+    }
+
     public addRoomListener(roomId: string, fn: Listener) {
         if (!this.roomIdActivityListeners[roomId]) this.roomIdActivityListeners[roomId] = [];
         this.roomIdActivityListeners[roomId].push(fn);
@@ -145,6 +155,14 @@ export class RoomViewStore extends Store<ActionPayload> {
         if (lastRoomId !== this.state.roomId) {
             if (lastRoomId) this.emitForRoom(lastRoomId, false);
             if (this.state.roomId) this.emitForRoom(this.state.roomId, true);
+
+            // Fired so we can reduce dependency on event emitters to this store, which is relatively
+            // central to the application and can easily cause import cycles.
+            dis.dispatch({
+                action: Action.ActiveRoomChanged,
+                oldRoomId: lastRoomId,
+                newRoomId: this.state.roomId,
+            } as ActiveRoomChangedPayload);
         }
 
         this.__emitChange();
@@ -197,7 +215,7 @@ export class RoomViewStore extends Store<ActionPayload> {
                     this.setState({ shouldPeek: false });
                 }
 
-                const cli = MatrixClientPeg.get();
+                const cli = this.matrixClient;
 
                 const updateMetrics = () => {
                     const room = cli.getRoom(payload.roomId);
@@ -280,7 +298,7 @@ export class RoomViewStore extends Store<ActionPayload> {
                     trigger: payload.metricsTrigger,
                     viaKeyboard: payload.metricsViaKeyboard,
                     isDM: !!DMRoomMap.shared().getUserIdForRoomId(payload.room_id),
-                    isSpace: MatrixClientPeg.get().getRoom(payload.room_id)?.isSpaceRoom(),
+                    isSpace: this.matrixClient.getRoom(payload.room_id)?.isSpaceRoom(),
                     activeSpace,
                 });
             }
@@ -339,7 +357,7 @@ export class RoomViewStore extends Store<ActionPayload> {
                     wasContextSwitch: payload.context_switch,
                 });
                 try {
-                    const result = await MatrixClientPeg.get().getRoomIdForAlias(payload.room_alias);
+                    const result = await this.matrixClient.getRoomIdForAlias(payload.room_alias);
                     storeRoomAliasInCache(payload.room_alias, result.room_id);
                     roomId = result.room_id;
                 } catch (err) {
@@ -376,7 +394,7 @@ export class RoomViewStore extends Store<ActionPayload> {
             joining: true,
         });
 
-        const cli = MatrixClientPeg.get();
+        const cli = this.matrixClient;
         // take a copy of roomAlias & roomId as they may change by the time the join is complete
         const { roomAlias, roomId } = this.state;
         const address = roomAlias || roomId;
@@ -408,7 +426,7 @@ export class RoomViewStore extends Store<ActionPayload> {
     }
 
     private getInvitingUserId(roomId: string): string {
-        const cli = MatrixClientPeg.get();
+        const cli = this.matrixClient;
         const room = cli.getRoom(roomId);
         if (room && room.getMyMembership() === "invite") {
             const myMember = room.getMember(cli.getUserId());
@@ -433,7 +451,7 @@ export class RoomViewStore extends Store<ActionPayload> {
             // only provide a better error message for invites
             if (invitingUserId) {
                 // if the inviting user is on the same HS, there can only be one cause: they left.
-                if (invitingUserId.endsWith(`:${MatrixClientPeg.get().getDomain()}`)) {
+                if (invitingUserId.endsWith(`:${this.matrixClient.getDomain()}`)) {
                     msg = _t("The person who invited you already left the room.");
                 } else {
                     msg = _t("The person who invited you already left the room, or their server is offline.");

src/stores/right-panel/RightPanelStore.ts

@@ -14,9 +14,9 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import { EventSubscription } from 'fbemitter';
 import { logger } from "matrix-js-sdk/src/logger";
 import { CryptoEvent } from "matrix-js-sdk/src/crypto";
+import { Optional } from "matrix-events-sdk";
 
 import defaultDispatcher from '../../dispatcher/dispatcher';
 import { pendingVerificationRequestForUser } from '../../verification';
@@ -31,7 +31,9 @@ import {
     IRightPanelCard,
     IRightPanelForRoom,
 } from './RightPanelStoreIPanelState';
-import { RoomViewStore } from '../RoomViewStore';
+import { ActionPayload } from "../../dispatcher/payloads";
+import { Action } from "../../dispatcher/actions";
+import { ActiveRoomChangedPayload } from "../../dispatcher/payloads/ActiveRoomChangedPayload";
 
 /**
  * A class for tracking the state of the right panel between layouts and
@@ -41,30 +43,32 @@ import { RoomViewStore } from '../RoomViewStore';
 */
 export default class RightPanelStore extends ReadyWatchingStore {
     private static internalInstance: RightPanelStore;
-    private viewedRoomId: string;
 
     private global?: IRightPanelForRoom = null;
     private byRoom: {
         [roomId: string]: IRightPanelForRoom;
     } = {};
-
+    private viewedRoomId: Optional<string>;
-    private roomStoreToken: EventSubscription;
 
     private constructor() {
         super(defaultDispatcher);
     }
 
     protected async onReady(): Promise<any> {
-        this.roomStoreToken = RoomViewStore.instance.addListener(this.onRoomViewStoreUpdate);
         this.matrixClient.on(CryptoEvent.VerificationRequest, this.onVerificationRequestUpdate);
-        this.viewedRoomId = RoomViewStore.instance.getRoomId();
         this.loadCacheFromSettings();
         this.emitAndUpdateSettings();
     }
 
     protected async onNotReady(): Promise<any> {
         this.matrixClient.off(CryptoEvent.VerificationRequest, this.onVerificationRequestUpdate);
-        this.roomStoreToken.remove();
+    }
+
+    protected onDispatcherAction(payload: ActionPayload) {
+        if (payload.action !== Action.ActiveRoomChanged) return;
+
+        const changePayload = <ActiveRoomChangedPayload>payload;
+        this.handleViewedRoomChange(changePayload.oldRoomId, changePayload.newRoomId);
     }
 
     // Getters
@@ -336,23 +340,20 @@ export default class RightPanelStore extends ReadyWatchingStore {
         }
     };
 
-    private onRoomViewStoreUpdate = () => {
+    private handleViewedRoomChange(oldRoomId: Optional<string>, newRoomId: Optional<string>) {
-        const oldRoomId = this.viewedRoomId;
+        this.viewedRoomId = newRoomId;
-        this.viewedRoomId = RoomViewStore.instance.getRoomId();
         // load values from byRoomCache with the viewedRoomId.
         this.loadCacheFromSettings();
 
-        // if we're switching to a room, clear out any stale MemberInfo cards
+        // when we're switching to a room, clear out any stale MemberInfo cards
         // in order to fix https://github.com/vector-im/element-web/issues/21487
-        if (oldRoomId !== this.viewedRoomId) {
+        if (this.currentCard?.phase !== RightPanelPhases.EncryptionPanel) {
-            if (this.currentCard?.phase !== RightPanelPhases.EncryptionPanel) {
+            const panel = this.byRoom[this.viewedRoomId];
-                const panel = this.byRoom[this.viewedRoomId];
+            if (panel?.history) {
-                if (panel?.history) {
+                panel.history = panel.history.filter(
-                    panel.history = panel.history.filter(
+                    (card) => card.phase != RightPanelPhases.RoomMemberInfo &&
-                        (card) => card.phase != RightPanelPhases.RoomMemberInfo &&
+                        card.phase != RightPanelPhases.Room3pidMemberInfo,
-                                  card.phase != RightPanelPhases.Room3pidMemberInfo,
+                );
-                    );
-                }
             }
         }
 
@@ -374,7 +375,7 @@ export default class RightPanelStore extends ReadyWatchingStore {
             };
         }
         this.emitAndUpdateSettings();
-    };
+    }
 
     private get isViewingRoom(): boolean {
         return !!this.viewedRoomId;

test/components/structures/RightPanel-test.tsx

@@ -32,6 +32,7 @@ import { RightPanelPhases } from "../../../src/stores/right-panel/RightPanelStor
 import RightPanelStore from "../../../src/stores/right-panel/RightPanelStore";
 import { UPDATE_EVENT } from "../../../src/stores/AsyncStore";
 import { WidgetLayoutStore } from "../../../src/stores/widgets/WidgetLayoutStore";
+import { RoomViewStore } from "../../../src/stores/RoomViewStore";
 
 describe("RightPanel", () => {
     it("renders info from only one room during room changes", async () => {
@@ -75,6 +76,7 @@ describe("RightPanel", () => {
         // @ts-ignore
         await WidgetLayoutStore.instance.onReady();
         RightPanelStore.instance.useUnitTestClient(cli);
+        RoomViewStore.instance.useUnitTestClient(cli);
         // @ts-ignore
         await RightPanelStore.instance.onReady();
 

test/components/views/elements/AppTile-test.tsx

@@ -36,6 +36,7 @@ import WidgetStore, { IApp } from "../../../../src/stores/WidgetStore";
 import AppTile from "../../../../src/components/views/elements/AppTile";
 import { Container, WidgetLayoutStore } from "../../../../src/stores/widgets/WidgetLayoutStore";
 import AppsDrawer from "../../../../src/components/views/rooms/AppsDrawer";
+import { RoomViewStore } from "../../../../src/stores/RoomViewStore";
 
 describe("AppTile", () => {
     let cli;
@@ -108,6 +109,7 @@ describe("AppTile", () => {
         // @ts-ignore
         await WidgetLayoutStore.instance.onReady();
         RightPanelStore.instance.useUnitTestClient(cli);
+        RoomViewStore.instance.useUnitTestClient(cli);
         // @ts-ignore
         await RightPanelStore.instance.onReady();
     });