Refactor stores and their relationship to the MatrixClientPeg (#124)

* Refactor stores and their relationship to the MatrixClientPeg

to avoid import cycles and webpack weirdness

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

src/MatrixClientPeg.ts

@@ -43,6 +43,7 @@ import { formatList } from "./utils/FormattingUtils";
 import SdkConfig from "./SdkConfig";
 import { Features } from "./settings/Settings";
 import { setDeviceIsolationMode } from "./settings/controllers/DeviceIsolationModeController.ts";
+import { ReadyWatchingStore } from "./stores/ReadyWatchingStore.ts";
 
 export interface IMatrixClientCreds {
     homeserverUrl: string;
@@ -309,6 +310,7 @@ class MatrixClientPegClass implements IMatrixClientPeg {
         MatrixActionCreators.start(this.matrixClient);
         MatrixClientBackedSettingsHandler.matrixClient = this.matrixClient;
         MatrixClientBackedController.matrixClient = this.matrixClient;
+        ReadyWatchingStore.matrixClient = this.matrixClient;
 
         return opts;
     }

src/stores/AsyncStoreWithClient.ts

@@ -36,8 +36,13 @@ export abstract class AsyncStoreWithClient<T extends Object> extends AsyncStore<
         })(dispatcher);
     }
 
-    public async start(): Promise<void> {
+    protected async start(matrixClient: MatrixClient | null): Promise<void> {
-        await this.readyStore.start();
+        await this.readyStore.start(matrixClient);
+    }
+
+    // XXX: This method is intended only for use in tests.
+    public async useUnitTestClient(cli: MatrixClient): Promise<void> {
+        await this.readyStore.useUnitTestClient(cli);
     }
 
     public get matrixClient(): MatrixClient | null {

src/stores/AutoRageshakeStore.ts

@@ -46,9 +46,7 @@ interface IState {
  */
 export default class AutoRageshakeStore extends AsyncStoreWithClient<IState> {
     private static readonly internalInstance = (() => {
-        const instance = new AutoRageshakeStore();
+        return new AutoRageshakeStore();
-        instance.start();
-        return instance;
     })();
 
     private constructor() {

src/stores/BreadcrumbsStore.ts

@@ -30,9 +30,7 @@ interface IState {
 
 export class BreadcrumbsStore extends AsyncStoreWithClient<IState> {
     private static readonly internalInstance = (() => {
-        const instance = new BreadcrumbsStore();
+        return new BreadcrumbsStore();
-        instance.start();
-        return instance;
     })();
 
     private waitingRooms: { roomId: string; addedTs: number }[] = [];

src/stores/CallStore.ts

@@ -31,7 +31,6 @@ export class CallStore extends AsyncStoreWithClient<{}> {
     public static get instance(): CallStore {
         if (!this._instance) {
             this._instance = new CallStore();
-            this._instance.start();
         }
         return this._instance;
     }

src/stores/ModalWidgetStore.ts

@@ -24,7 +24,6 @@ interface IState {
 export class ModalWidgetStore extends AsyncStoreWithClient<IState> {
     private static readonly internalInstance = (() => {
         const instance = new ModalWidgetStore();
-        instance.start();
         return instance;
     })();
     private modalInstance: IHandle<typeof ModalWidgetDialog> | null = null;

src/stores/OwnBeaconStore.ts

@@ -87,7 +87,6 @@ const getLocallyCreatedBeaconEventIds = (): string[] => {
 export class OwnBeaconStore extends AsyncStoreWithClient<OwnBeaconStoreState> {
     private static readonly internalInstance = (() => {
         const instance = new OwnBeaconStore();
-        instance.start();
         return instance;
     })();
     // users beacons, keyed by event type

src/stores/OwnProfileStore.ts

@@ -28,7 +28,6 @@ const KEY_AVATAR_URL = "mx_profile_avatar_url";
 export class OwnProfileStore extends AsyncStoreWithClient<IState> {
     private static readonly internalInstance = (() => {
         const instance = new OwnProfileStore();
-        instance.start();
         return instance;
     })();
 

src/stores/ReadyWatchingStore.ts

@@ -9,27 +9,40 @@
 import { MatrixClient, SyncState } from "matrix-js-sdk/src/matrix";
 import { EventEmitter } from "events";
 
-import { MatrixClientPeg } from "../MatrixClientPeg";
 import { ActionPayload } from "../dispatcher/payloads";
 import { IDestroyable } from "../utils/IDestroyable";
 import { Action } from "../dispatcher/actions";
 import { MatrixDispatcher } from "../dispatcher/dispatcher";
 
 export abstract class ReadyWatchingStore extends EventEmitter implements IDestroyable {
-    protected matrixClient: MatrixClient | null = null;
+    private static instances: ReadyWatchingStore[] = [];
+    protected _matrixClient: MatrixClient | null = null;
     private dispatcherRef: string | null = null;
 
+    public static set matrixClient(client: MatrixClient) {
+        for (const instance of ReadyWatchingStore.instances) {
+            instance.start(client);
+        }
+    }
+
     public constructor(protected readonly dispatcher: MatrixDispatcher) {
         super();
+
+        this.dispatcherRef = this.dispatcher.register(this.onAction);
     }
 
-    public async start(): Promise<void> {
+    public get matrixClient(): MatrixClient | null {
-        this.dispatcherRef = this.dispatcher.register(this.onAction);
+        return this._matrixClient;
+    }
 
-        // MatrixClientPeg can be undefined in tests because of circular dependencies with other stores
+    public async start(matrixClient: MatrixClient | null): Promise<void> {
-        const matrixClient = MatrixClientPeg?.get();
+        const oldClient = this._matrixClient;
+        this._matrixClient = matrixClient;
+
+        if (oldClient !== matrixClient) {
+            await this.onNotReady();
+        }
         if (matrixClient) {
-            this.matrixClient = matrixClient;
             await this.onReady();
         }
     }
@@ -38,8 +51,10 @@ export abstract class ReadyWatchingStore extends EventEmitter implements IDestro
         return this.matrixClient; // for external readonly access
     }
 
-    public useUnitTestClient(cli: MatrixClient): void {
+    // XXX: This method is intended only for use in tests.
-        this.matrixClient = cli;
+    public async useUnitTestClient(cli: MatrixClient): Promise<void> {
+        this._matrixClient = cli;
+        await this.onReady();
     }
 
     public destroy(): void {
@@ -74,13 +89,13 @@ export abstract class ReadyWatchingStore extends EventEmitter implements IDestro
                 if (this.matrixClient) {
                     await this.onNotReady();
                 }
-                this.matrixClient = payload.matrixClient;
+                this._matrixClient = payload.matrixClient;
                 await this.onReady();
             }
         } else if (payload.action === "on_client_not_viable" || payload.action === Action.OnLoggedOut) {
             if (this.matrixClient) {
                 await this.onNotReady();
-                this.matrixClient = null;
+                this._matrixClient = null;
             }
         }
     };

src/stores/VoiceRecordingStore.ts

@@ -30,7 +30,6 @@ export class VoiceRecordingStore extends AsyncStoreWithClient<IState> {
     public static get instance(): VoiceRecordingStore {
         if (!this.internalInstance) {
             this.internalInstance = new VoiceRecordingStore();
-            this.internalInstance.start();
         }
         return this.internalInstance;
     }

src/stores/WidgetStore.ts

@@ -45,7 +45,6 @@ interface IRoomWidgets {
 export default class WidgetStore extends AsyncStoreWithClient<IState> {
     private static readonly internalInstance = (() => {
         const instance = new WidgetStore();
-        instance.start();
         return instance;
     })();
 

src/stores/local-echo/EchoStore.ts

@@ -38,7 +38,6 @@ export class EchoStore extends AsyncStoreWithClient<IState> {
     public static get instance(): EchoStore {
         if (!this._instance) {
             this._instance = new EchoStore();
-            this._instance.start();
         }
         return this._instance;
     }

src/stores/notifications/RoomNotificationStateStore.ts

@@ -26,7 +26,6 @@ export const UPDATE_STATUS_INDICATOR = Symbol("update-status-indicator");
 export class RoomNotificationStateStore extends AsyncStoreWithClient<IState> {
     private static readonly internalInstance = (() => {
         const instance = new RoomNotificationStateStore();
-        instance.start();
         return instance;
     })();
     private roomMap = new Map<Room, RoomNotificationState>();

src/stores/right-panel/RightPanelStore.ts

@@ -403,7 +403,6 @@ export default class RightPanelStore extends ReadyWatchingStore {
     public static get instance(): RightPanelStore {
         if (!this.internalInstance) {
             this.internalInstance = new RightPanelStore();
-            this.internalInstance.start();
         }
         return this.internalInstance;
     }

src/stores/room-list/MessagePreviewStore.ts

@@ -124,11 +124,7 @@ const mkMessagePreview = (text: string, event: MatrixEvent): MessagePreview => {
 };
 
 export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
-    private static readonly internalInstance = (() => {
+    private static readonly internalInstance = (() => new MessagePreviewStore())();
-        const instance = new MessagePreviewStore();
-        instance.start();
-        return instance;
-    })();
 
     /**
      * @internal Public for test only

src/stores/room-list/RoomListLayoutStore.ts

@@ -28,7 +28,6 @@ export default class RoomListLayoutStore extends AsyncStoreWithClient<IState> {
     public static get instance(): RoomListLayoutStore {
         if (!this.internalInstance) {
             this.internalInstance = new RoomListLayoutStore();
-            this.internalInstance.start();
         }
         return RoomListLayoutStore.internalInstance;
     }

src/stores/room-list/RoomListStore.ts

@@ -643,11 +643,9 @@ export default class RoomListStore {
             if (SettingsStore.getValue("feature_sliding_sync")) {
                 logger.info("using SlidingRoomListStoreClass");
                 const instance = new SlidingRoomListStoreClass(defaultDispatcher, SdkContextClass.instance);
-                instance.start();
                 RoomListStore.internalInstance = instance;
             } else {
                 const instance = new RoomListStoreClass(defaultDispatcher);
-                instance.start();
                 RoomListStore.internalInstance = instance;
             }
         }

src/stores/spaces/SpaceStore.ts

@@ -17,6 +17,7 @@ import {
     MatrixEvent,
     ClientEvent,
     ISendEventResponse,
+    MatrixClient,
 } from "matrix-js-sdk/src/matrix";
 import { KnownMembership } from "matrix-js-sdk/src/types";
 import { logger } from "matrix-js-sdk/src/logger";
@@ -1397,7 +1398,6 @@ export class SpaceStoreClass extends AsyncStoreWithClient<IState> {
 export default class SpaceStore {
     private static readonly internalInstance = (() => {
         const instance = new SpaceStoreClass();
-        instance.start();
         return instance;
     })();
 
@@ -1408,9 +1408,9 @@ export default class SpaceStore {
     /**
      * @internal for test only
      */
-    public static testInstance(): SpaceStoreClass {
+    public static testInstance(client: MatrixClient): SpaceStoreClass {
         const store = new SpaceStoreClass();
-        store.start();
+        store.useUnitTestClient(client);
         return store;
     }
 }

src/stores/widgets/WidgetLayoutStore.ts

@@ -60,7 +60,6 @@ export class WidgetLayoutStore extends ReadyWatchingStore {
     public static get instance(): WidgetLayoutStore {
         if (!this.internalInstance) {
             this.internalInstance = new WidgetLayoutStore();
-            this.internalInstance.start();
         }
         return this.internalInstance;
     }

src/stores/widgets/WidgetMessagingStore.ts

@@ -27,7 +27,6 @@ export enum WidgetMessagingStoreEvent {
 export class WidgetMessagingStore extends AsyncStoreWithClient<{}> {
     private static readonly internalInstance = (() => {
         const instance = new WidgetMessagingStore();
-        instance.start();
         return instance;
     })();
 

test/stores/AutoRageshakeStore-test.ts

@@ -47,7 +47,7 @@ describe("AutoRageshakeStore", () => {
 
         // @ts-ignore bypass private ctor for tests
         autoRageshakeStore = new AutoRageshakeStore();
-        autoRageshakeStore.start();
+        autoRageshakeStore.useUnitTestClient(client);
 
         utdEvent = mkEvent({
             event: true,

test/stores/OwnProfileStore-test.ts

@@ -40,7 +40,7 @@ describe("OwnProfileStore", () => {
             displayname: "Display Name",
             avatar_url: "mxc://example.com/abc123",
         });
-        await ownProfileStore.start();
+        await ownProfileStore.useUnitTestClient(client);
 
         expect(onUpdate).toHaveBeenCalled();
         expect(ownProfileStore.displayName).toBe("Display Name");
@@ -54,7 +54,7 @@ describe("OwnProfileStore", () => {
                 errcode: "M_NOT_FOUND",
             }),
         );
-        await ownProfileStore.start();
+        await ownProfileStore.useUnitTestClient(client);
 
         expect(onUpdate).toHaveBeenCalled();
         expect(ownProfileStore.displayName).toBe(client.getSafeUserId());
@@ -69,7 +69,7 @@ describe("OwnProfileStore", () => {
             }),
         );
         try {
-            await ownProfileStore.start();
+            await ownProfileStore.useUnitTestClient(client);
         } catch (ignore) {}
 
         expect(onUpdate).not.toHaveBeenCalled();

test/stores/SpaceStore-test.ts

@@ -1428,7 +1428,7 @@ describe("SpaceStore", () => {
 
         it("passes that value in calls to getVisibleRooms during getSpaceFilteredRoomIds", () => {
             // Given a store
-            const store = SpaceStore.testInstance();
+            const store = SpaceStore.testInstance(client);
 
             // When we ask for filtered room ids
             store.getSpaceFilteredRoomIds(MetaSpace.Home);
@@ -1478,7 +1478,7 @@ describe("SpaceStore", () => {
 
         it("passes that value in calls to getVisibleRooms during getSpaceFilteredRoomIds", () => {
             // Given a store
-            const store = SpaceStore.testInstance();
+            const store = SpaceStore.testInstance(client);
             // When we ask for filtered room ids
             store.getSpaceFilteredRoomIds(MetaSpace.Home);
 

test/stores/VoiceRecordingStore-test.ts

@@ -31,7 +31,7 @@ describe("VoiceRecordingStore", () => {
 
     const mkStore = (): VoiceRecordingStore => {
         const store = new VoiceRecordingStore();
-        store.start();
+        store.useUnitTestClient(stubClient);
         return store;
     };
 

test/stores/WidgetLayoutStore-test.ts

@@ -167,7 +167,7 @@ describe("WidgetLayoutStore", () => {
 
     it("should recalculate all rooms when the client is ready", async () => {
         mocked(client.getVisibleRooms).mockReturnValue([mockRoom]);
-        await store.start();
+        await store.start(client);
 
         expect(roomUpdateListener).toHaveBeenCalled();
         expect(store.getContainerWidgets(mockRoom, Container.Top)).toEqual([]);
@@ -243,7 +243,7 @@ describe("WidgetLayoutStore", () => {
     });
 
     it("should copy the layout to the room", async () => {
-        await store.start();
+        await store.start(client);
         store.recalculateRoom(mockRoom);
         store.moveToContainer(mockRoom, mockApps[0], Container.Top);
         store.copyLayoutToRoom(mockRoom);
@@ -297,7 +297,7 @@ describe("WidgetLayoutStore", () => {
             mocked(client.getVisibleRooms).mockReturnValue([]);
             // @ts-ignore bypass private ctor for tests
             const store = new WidgetLayoutStore();
-            await store.start();
+            await store.start(client);
             expect(client.getVisibleRooms).toHaveBeenCalledWith(false);
         });
     });
@@ -314,7 +314,7 @@ describe("WidgetLayoutStore", () => {
             mocked(client.getVisibleRooms).mockReturnValue([]);
             // @ts-ignore bypass private ctor for tests
             const store = new WidgetLayoutStore();
-            await store.start();
+            await store.start(client);
             expect(client.getVisibleRooms).toHaveBeenCalledWith(true);
         });
     });

test/stores/room-list/MessagePreviewStore-test.ts

@@ -80,7 +80,7 @@ describe("MessagePreviewStore", () => {
         mocked(client.getRoom).mockReturnValue(room);
 
         store = MessagePreviewStore.testInstance();
-        await store.start();
+        await store.useUnitTestClient(client);
         await setupAsyncStoreWithClient(store, client);
     });
 

test/stores/room-list/RoomListStore-test.ts

@@ -29,6 +29,9 @@ import DMRoomMap from "../../../src/utils/DMRoomMap";
 import { flushPromises, stubClient, upsertRoomStateEvents, mkRoom } from "../../test-utils";
 import { DEFAULT_PUSH_RULES, makePushRule } from "../../test-utils/pushRules";
 
+// Mock out the SpaceWatcher as it messes with the prefilterConditions
+jest.mock("../../../src/stores/room-list/SpaceWatcher.ts");
+
 describe("RoomListStore", () => {
     const client = stubClient();
     const newRoomId = "!roomid:example.com";
@@ -91,6 +94,10 @@ describe("RoomListStore", () => {
         await (RoomListStore.instance as RoomListStoreClass).makeReady(client);
     });
 
+    beforeEach(() => {
+        DMRoomMap.makeShared(client);
+    });
+
     it.each(OrderedDefaultTagIDs)("defaults to importance ordering for %s=", (tagId) => {
         expect(RoomListStore.instance.getTagSorting(tagId)).toBe(SortAlgorithm.Recent);
     });
@@ -102,11 +109,11 @@ describe("RoomListStore", () => {
     function createStore(): { store: RoomListStoreClass; handleRoomUpdate: jest.Mock<any, any> } {
         const fakeDispatcher = { register: jest.fn() } as unknown as MatrixDispatcher;
         const store = new RoomListStoreClass(fakeDispatcher);
-        // @ts-ignore accessing private member to set client
-        store.readyStore.matrixClient = client;
         const handleRoomUpdate = jest.fn();
         // @ts-ignore accessing private member to mock it
         store.algorithm.handleRoomUpdate = handleRoomUpdate;
+        // @ts-ignore accessing private member to set client
+        store.readyStore.useUnitTestClient(client);
 
         return { store, handleRoomUpdate };
     }
@@ -157,7 +164,6 @@ describe("RoomListStore", () => {
         room1.updateMyMembership(KnownMembership.Join);
         room2.updateMyMembership(KnownMembership.Join);
         room3.updateMyMembership(KnownMembership.Join);
-        DMRoomMap.makeShared(client);
         const { store } = createStore();
         client.getVisibleRooms = jest.fn().mockReturnValue([room1, room2, room3]);
 
@@ -269,7 +275,6 @@ describe("RoomListStore", () => {
 
         it("Passes the feature flag on to the client when asking for visible rooms", () => {
             // Given a store that we can ask for a room list
-            DMRoomMap.makeShared(client);
             const { store } = createStore();
             client.getVisibleRooms = jest.fn().mockReturnValue([]);
 
@@ -285,7 +290,7 @@ describe("RoomListStore", () => {
     describe("room updates", () => {
         const makeStore = async () => {
             const store = new RoomListStoreClass(defaultDispatcher);
-            await store.start();
+            await store.useUnitTestClient(client);
             return store;
         };
 

test/stores/room-list/SlidingRoomListStore-test.ts

@@ -65,7 +65,7 @@ describe("SlidingRoomListStore", () => {
 
     describe("spaces", () => {
         it("alters 'filters.spaces' on the DefaultTagID.Untagged list when the selected space changes", async () => {
-            await store.start(); // call onReady
+            await store.useUnitTestClient(context.client!); // call onReady
             const spaceRoomId = "!foo:bar";
 
             const p = untilEmission(store, LISTS_LOADING_EVENT, (listName, isLoading) => {
@@ -92,7 +92,7 @@ describe("SlidingRoomListStore", () => {
                 },
             );
             activeSpace = MetaSpace.Home;
-            await store.start(); // call onReady
+            await store.useUnitTestClient(context.client!); // call onReady
 
             expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(DefaultTagID.Untagged, {
                 filters: expect.objectContaining({
@@ -108,7 +108,7 @@ describe("SlidingRoomListStore", () => {
             const p = untilEmission(store, LISTS_LOADING_EVENT, (listName, isLoading) => {
                 return listName === DefaultTagID.Untagged && !isLoading;
             });
-            await store.start(); // call onReady
+            await store.useUnitTestClient(context.client!); // call onReady
             await p;
             expect(context._SlidingSyncManager!.ensureListRegistered).toHaveBeenCalledWith(
                 DefaultTagID.Untagged,
@@ -121,7 +121,7 @@ describe("SlidingRoomListStore", () => {
         });
 
         it("includes subspaces in 'filters.spaces' when the selected space has subspaces", async () => {
-            await store.start(); // call onReady
+            await store.useUnitTestClient(context.client!); // call onReady
             const spaceRoomId = "!foo:bar";
             const subSpace1 = "!ss1:bar";
             const subSpace2 = "!ss2:bar";
@@ -168,7 +168,7 @@ describe("SlidingRoomListStore", () => {
     });
 
     it("getTagsForRoom gets the tags for the room", async () => {
-        await store.start();
+        await store.useUnitTestClient(context.client!);
         const roomA = "!a:localhost";
         const roomB = "!b:localhost";
         const keyToListData: Record<string, { joinedCount: number; roomIndexToRoomId: Record<number, string> }> = {
@@ -200,7 +200,7 @@ describe("SlidingRoomListStore", () => {
     });
 
     it("emits LISTS_UPDATE_EVENT when slidingSync lists update", async () => {
-        await store.start();
+        await store.useUnitTestClient(context.client!);
         const roomA = "!a:localhost";
         const roomB = "!b:localhost";
         const roomC = "!c:localhost";
@@ -236,7 +236,7 @@ describe("SlidingRoomListStore", () => {
     });
 
     it("sets the sticky room on the basis of the viewed room in RoomViewStore", async () => {
-        await store.start();
+        await store.useUnitTestClient(context.client!);
         // seed the store with 3 rooms
         const roomIdA = "!a:localhost";
         const roomIdB = "!b:localhost";
@@ -301,7 +301,7 @@ describe("SlidingRoomListStore", () => {
     });
 
     it("gracefully handles unknown room IDs", async () => {
-        await store.start();
+        await store.useUnitTestClient(context.client!);
         const roomIdA = "!a:localhost";
         const roomIdB = "!b:localhost"; // does not exist
         const roomIdC = "!c:localhost";