Revert #124 and #135 (#139)

This seems to be causing a lot of weirdness, presumably because there's
another missing thing like in #135, but I don't know what it might be and
it feels like it might take a while to find. Backing these changes out
to fix develop while we sort it out.

Fixes element-hq/element-web#28179

Author: dbkr

src/MatrixClientPeg.ts

@@ -43,7 +43,6 @@ 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;
@@ -310,7 +309,6 @@ 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,13 +36,8 @@ export abstract class AsyncStoreWithClient<T extends Object> extends AsyncStore<
         })(dispatcher);
     }
 
-    protected async start(matrixClient: MatrixClient | null): Promise<void> {
-        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 async start(): Promise<void> {
+        await this.readyStore.start();
     }
 
     public get matrixClient(): MatrixClient | null {

src/stores/AutoRageshakeStore.ts

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

src/stores/BreadcrumbsStore.ts

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

src/stores/CallStore.ts

@@ -31,6 +31,7 @@ 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,6 +24,7 @@ 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,6 +87,7 @@ 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,6 +28,7 @@ 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,42 +9,27 @@
 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 {
-    private static instances: ReadyWatchingStore[] = [];
-    protected _matrixClient: MatrixClient | null = null;
+    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);
-
-        ReadyWatchingStore.instances.push(this);
-    }
-
-    public get matrixClient(): MatrixClient | null {
-        return this._matrixClient;
     }
 
-    public async start(matrixClient: MatrixClient | null): Promise<void> {
-        const oldClient = this._matrixClient;
-        this._matrixClient = matrixClient;
+    public async start(): Promise<void> {
+        this.dispatcherRef = this.dispatcher.register(this.onAction);
 
-        if (oldClient !== matrixClient) {
-            await this.onNotReady();
-        }
+        // MatrixClientPeg can be undefined in tests because of circular dependencies with other stores
+        const matrixClient = MatrixClientPeg?.get();
         if (matrixClient) {
+            this.matrixClient = matrixClient;
             await this.onReady();
         }
     }
@@ -53,10 +38,8 @@ export abstract class ReadyWatchingStore extends EventEmitter implements IDestro
         return this.matrixClient; // for external readonly access
     }
 
-    // XXX: This method is intended only for use in tests.
-    public async useUnitTestClient(cli: MatrixClient): Promise<void> {
-        this._matrixClient = cli;
-        await this.onReady();
+    public useUnitTestClient(cli: MatrixClient): void {
+        this.matrixClient = cli;
     }
 
     public destroy(): void {
@@ -91,13 +74,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,6 +30,7 @@ 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,6 +45,7 @@ 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,6 +38,7 @@ 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,6 +26,7 @@ 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,6 +403,7 @@ 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,7 +124,11 @@ const mkMessagePreview = (text: string, event: MatrixEvent): MessagePreview => {
 };
 
 export class MessagePreviewStore extends AsyncStoreWithClient<IState> {
-    private static readonly internalInstance = (() => new MessagePreviewStore())();
+    private static readonly internalInstance = (() => {
+        const instance = new MessagePreviewStore();
+        instance.start();
+        return instance;
+    })();
 
     /**
      * @internal Public for test only

src/stores/room-list/RoomListLayoutStore.ts

@@ -28,6 +28,7 @@ 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,9 +643,11 @@ 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,7 +17,6 @@ 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";
@@ -1398,6 +1397,7 @@ 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(client: MatrixClient): SpaceStoreClass {
+    public static testInstance(): SpaceStoreClass {
         const store = new SpaceStoreClass();
-        store.useUnitTestClient(client);
+        store.start();
         return store;
     }
 }

src/stores/widgets/WidgetLayoutStore.ts

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

test/MatrixClientPeg-test.ts

@@ -6,8 +6,8 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
 Please see LICENSE files in the repository root for full details.
 */
 
-import * as MatrixJs from "matrix-js-sdk/src/matrix";
 import { logger } from "matrix-js-sdk/src/logger";
+import fetchMockJest from "fetch-mock-jest";
 
 import { advanceDateAndTime, stubClient } from "./test-utils";
 import { IMatrixClientPeg, MatrixClientPeg as peg } from "../src/MatrixClientPeg";
@@ -19,14 +19,9 @@ jest.useFakeTimers();
 const PegClass = Object.getPrototypeOf(peg).constructor;
 
 describe("MatrixClientPeg", () => {
-    let mockClient: MatrixJs.MatrixClient;
-
     beforeEach(() => {
         // stub out Logger.log which gets called a lot and clutters up the test output
         jest.spyOn(logger, "log").mockImplementation(() => {});
-
-        mockClient = stubClient();
-        jest.spyOn(MatrixJs, "createClient").mockReturnValue(mockClient);
     });
 
     afterEach(() => {
@@ -38,6 +33,7 @@ describe("MatrixClientPeg", () => {
     });
 
     it("setJustRegisteredUserId", () => {
+        stubClient();
         (peg as any).matrixClient = peg.get();
         peg.setJustRegisteredUserId("@userId:matrix.org");
         expect(peg.safeGet().credentials.userId).toBe("@userId:matrix.org");
@@ -56,6 +52,7 @@ describe("MatrixClientPeg", () => {
     });
 
     it("setJustRegisteredUserId(null)", () => {
+        stubClient();
         (peg as any).matrixClient = peg.get();
         peg.setJustRegisteredUserId(null);
         expect(peg.currentUserIsJustRegistered()).toBe(false);
@@ -74,6 +71,7 @@ describe("MatrixClientPeg", () => {
         beforeEach(() => {
             // instantiate a MatrixClientPegClass instance, with a new MatrixClient
             testPeg = new PegClass();
+            fetchMockJest.get("http://example.com/_matrix/client/versions", {});
             testPeg.replaceUsingCreds({
                 accessToken: "SEKRET",
                 homeserverUrl: "http://example.com",
@@ -85,20 +83,24 @@ describe("MatrixClientPeg", () => {
         it("should initialise the rust crypto library by default", async () => {
             const mockSetValue = jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined);
 
+            const mockInitCrypto = jest.spyOn(testPeg.safeGet(), "initCrypto").mockResolvedValue(undefined);
+            const mockInitRustCrypto = jest.spyOn(testPeg.safeGet(), "initRustCrypto").mockResolvedValue(undefined);
+
             const cryptoStoreKey = new Uint8Array([1, 2, 3, 4]);
             await testPeg.start({ rustCryptoStoreKey: cryptoStoreKey });
-            expect(mockClient.initCrypto).not.toHaveBeenCalled();
-            expect(mockClient.initRustCrypto).toHaveBeenCalledWith({ storageKey: cryptoStoreKey });
+            expect(mockInitCrypto).not.toHaveBeenCalled();
+            expect(mockInitRustCrypto).toHaveBeenCalledWith({ storageKey: cryptoStoreKey });
 
             // we should have stashed the setting in the settings store
             expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, true);
         });
 
         it("Should migrate existing login", async () => {
             const mockSetValue = jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined);
+            const mockInitRustCrypto = jest.spyOn(testPeg.safeGet(), "initRustCrypto").mockResolvedValue(undefined);
 
             await testPeg.start();
-            expect(mockClient.initRustCrypto).toHaveBeenCalledTimes(1);
+            expect(mockInitRustCrypto).toHaveBeenCalledTimes(1);
 
             // we should have stashed the setting in the settings store
             expect(mockSetValue).toHaveBeenCalledWith("feature_rust_crypto", null, SettingLevel.DEVICE, true);

test/stores/AutoRageshakeStore-test.ts

@@ -47,7 +47,7 @@ describe("AutoRageshakeStore", () => {
 
         // @ts-ignore bypass private ctor for tests
         autoRageshakeStore = new AutoRageshakeStore();
-        autoRageshakeStore.useUnitTestClient(client);
+        autoRageshakeStore.start();
 
         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.useUnitTestClient(client);
+        await ownProfileStore.start();
 
         expect(onUpdate).toHaveBeenCalled();
         expect(ownProfileStore.displayName).toBe("Display Name");
@@ -54,7 +54,7 @@ describe("OwnProfileStore", () => {
                 errcode: "M_NOT_FOUND",
             }),
         );
-        await ownProfileStore.useUnitTestClient(client);
+        await ownProfileStore.start();
 
         expect(onUpdate).toHaveBeenCalled();
         expect(ownProfileStore.displayName).toBe(client.getSafeUserId());
@@ -69,7 +69,7 @@ describe("OwnProfileStore", () => {
             }),
         );
         try {
-            await ownProfileStore.useUnitTestClient(client);
+            await ownProfileStore.start();
         } catch (ignore) {}
 
         expect(onUpdate).not.toHaveBeenCalled();