Move subscriptions out of RoomView constructor

Components are not 'cleaned up' (i.e. `componentWillUnmount` called) unless
`componentDidMount` was called. React doesn't expect to have to simulate
an unmount if the component was never mounted to begin with.

In the case of RoomView, that means that the subscriptions created in the
constructor are never removed, i.e. a leak.

This moves the subscriptions to the `componentDidMount` method.

See element-hq/element-web#24236 (comment)

Signed-off-by: Clark Fischer <clark.fischer@gmail.com>

Author: clarkf

src/components/structures/RoomView.tsx

@@ -1,8 +1,5 @@
 /*
-Copyright 2015, 2016 OpenMarket Ltd
-Copyright 2017 Vector Creations Ltd
-Copyright 2018, 2019 New Vector Ltd
-Copyright 2019 - 2022 The Matrix.org Foundation C.I.C.
+Copyright 2015, 2017 - 2023 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.
@@ -33,6 +30,7 @@ import { CryptoEvent } from "matrix-js-sdk/src/crypto";
 import { THREAD_RELATION_TYPE } from "matrix-js-sdk/src/models/thread";
 import { HistoryVisibility } from "matrix-js-sdk/src/@types/partials";
 import { ISearchResults } from "matrix-js-sdk/src/@types/search";
+import { IRoomTimelineData } from "matrix-js-sdk/src/matrix";
 
 import shouldHideEvent from "../../shouldHideEvent";
 import { _t } from "../../languageHandler";
@@ -360,8 +358,8 @@ function LocalRoomCreateLoader(props: ILocalRoomCreateLoaderProps): ReactElement
 }
 
 export class RoomView extends React.Component<IRoomProps, IRoomState> {
-    private readonly dispatcherRef: string;
-    private settingWatchers: string[];
+    private dispatcherRef: string | undefined = undefined;
+    private settingWatchers: string[] = [];
 
     private unmounted = false;
     private permalinkCreators: Record<string, RoomPermalinkCreator> = {};
@@ -417,57 +415,6 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
             narrow: false,
             visibleDecryptionFailures: [],
         };
-
-        this.dispatcherRef = dis.register(this.onAction);
-        context.client.on(ClientEvent.Room, this.onRoom);
-        context.client.on(RoomEvent.Timeline, this.onRoomTimeline);
-        context.client.on(RoomEvent.TimelineReset, this.onRoomTimelineReset);
-        context.client.on(RoomEvent.Name, this.onRoomName);
-        context.client.on(RoomStateEvent.Events, this.onRoomStateEvents);
-        context.client.on(RoomStateEvent.Update, this.onRoomStateUpdate);
-        context.client.on(RoomEvent.MyMembership, this.onMyMembership);
-        context.client.on(CryptoEvent.KeyBackupStatus, this.onKeyBackupStatus);
-        context.client.on(CryptoEvent.DeviceVerificationChanged, this.onDeviceVerificationChanged);
-        context.client.on(CryptoEvent.UserTrustStatusChanged, this.onUserVerificationChanged);
-        context.client.on(CryptoEvent.KeysChanged, this.onCrossSigningKeysChanged);
-        context.client.on(MatrixEventEvent.Decrypted, this.onEventDecrypted);
-        // Start listening for RoomViewStore updates
-        context.roomViewStore.on(UPDATE_EVENT, this.onRoomViewStoreUpdate);
-
-        context.rightPanelStore.on(UPDATE_EVENT, this.onRightPanelStoreUpdate);
-
-        WidgetEchoStore.on(UPDATE_EVENT, this.onWidgetEchoStoreUpdate);
-        context.widgetStore.on(UPDATE_EVENT, this.onWidgetStoreUpdate);
-
-        CallStore.instance.on(CallStoreEvent.ActiveCalls, this.onActiveCalls);
-
-        this.props.resizeNotifier.on("isResizing", this.onIsResizing);
-
-        this.settingWatchers = [
-            SettingsStore.watchSetting("layout", null, (...[, , , value]) =>
-                this.setState({ layout: value as Layout }),
-            ),
-            SettingsStore.watchSetting("lowBandwidth", null, (...[, , , value]) =>
-                this.setState({ lowBandwidth: value as boolean }),
-            ),
-            SettingsStore.watchSetting("alwaysShowTimestamps", null, (...[, , , value]) =>
-                this.setState({ alwaysShowTimestamps: value as boolean }),
-            ),
-            SettingsStore.watchSetting("showTwelveHourTimestamps", null, (...[, , , value]) =>
-                this.setState({ showTwelveHourTimestamps: value as boolean }),
-            ),
-            SettingsStore.watchSetting("readMarkerInViewThresholdMs", null, (...[, , , value]) =>
-                this.setState({ readMarkerInViewThresholdMs: value as number }),
-            ),
-            SettingsStore.watchSetting("readMarkerOutOfViewThresholdMs", null, (...[, , , value]) =>
-                this.setState({ readMarkerOutOfViewThresholdMs: value as number }),
-            ),
-            SettingsStore.watchSetting("showHiddenEventsInTimeline", null, (...[, , , value]) =>
-                this.setState({ showHiddenEvents: value as boolean }),
-            ),
-            SettingsStore.watchSetting("urlPreviewsEnabled", null, this.onUrlPreviewsEnabledChange),
-            SettingsStore.watchSetting("urlPreviewsEnabled_e2ee", null, this.onUrlPreviewsEnabledChange),
-        ];
     }
 
     private onIsResizing = (resizing: boolean): void => {
@@ -621,7 +568,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
         }
 
         // Add watchers for each of the settings we just looked up
-        this.settingWatchers = this.settingWatchers.concat([
+        this.settingWatchers.push(
             SettingsStore.watchSetting("showReadReceipts", roomId, (...[, , , value]) =>
                 this.setState({ showReadReceipts: value as boolean }),
             ),
@@ -637,7 +584,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
             SettingsStore.watchSetting("showDisplaynameChanges", roomId, (...[, , , value]) =>
                 this.setState({ showDisplaynameChanges: value as boolean }),
             ),
-        ]);
+        );
 
         if (!initial && this.state.shouldPeek && !newState.shouldPeek) {
             // Stop peeking because we have joined this room now
@@ -765,8 +712,8 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
                     peekLoading: true,
                     isPeeking: true, // this will change to false if peeking fails
                 });
-                this.context.client
-                    .peekInRoom(roomId)
+                this.context
+                    .client!.peekInRoom(roomId)
                     .then((room) => {
                         if (this.unmounted) {
                             return;
@@ -801,7 +748,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
                     });
             } else if (room) {
                 // Stop peeking because we have joined this room previously
-                this.context.client.stopPeeking();
+                this.context.client!.stopPeeking();
                 this.setState({ isPeeking: false });
             }
         }
@@ -824,6 +771,59 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
     }
 
     public componentDidMount(): void {
+        const context = this.context;
+        const client = context.client!;
+        this.dispatcherRef = dis.register(this.onAction);
+        client.on(ClientEvent.Room, this.onRoom);
+        client.on(RoomEvent.Timeline, this.onRoomTimeline);
+        client.on(RoomEvent.TimelineReset, this.onRoomTimelineReset);
+        client.on(RoomEvent.Name, this.onRoomName);
+        client.on(RoomStateEvent.Events, this.onRoomStateEvents);
+        client.on(RoomStateEvent.Update, this.onRoomStateUpdate);
+        client.on(RoomEvent.MyMembership, this.onMyMembership);
+        client.on(CryptoEvent.KeyBackupStatus, this.onKeyBackupStatus);
+        client.on(CryptoEvent.DeviceVerificationChanged, this.onDeviceVerificationChanged);
+        client.on(CryptoEvent.UserTrustStatusChanged, this.onUserVerificationChanged);
+        client.on(CryptoEvent.KeysChanged, this.onCrossSigningKeysChanged);
+        client.on(MatrixEventEvent.Decrypted, this.onEventDecrypted);
+        // Start listening for RoomViewStore updates
+        context.roomViewStore.on(UPDATE_EVENT, this.onRoomViewStoreUpdate);
+
+        context.rightPanelStore.on(UPDATE_EVENT, this.onRightPanelStoreUpdate);
+
+        WidgetEchoStore!.on(UPDATE_EVENT, this.onWidgetEchoStoreUpdate);
+        context.widgetStore.on(UPDATE_EVENT, this.onWidgetStoreUpdate);
+
+        CallStore.instance.on(CallStoreEvent.ActiveCalls, this.onActiveCalls);
+
+        this.props.resizeNotifier.on("isResizing", this.onIsResizing);
+
+        this.settingWatchers = [
+            SettingsStore.watchSetting("layout", null, (...[, , , value]) =>
+                this.setState({ layout: value as Layout }),
+            ),
+            SettingsStore.watchSetting("lowBandwidth", null, (...[, , , value]) =>
+                this.setState({ lowBandwidth: value as boolean }),
+            ),
+            SettingsStore.watchSetting("alwaysShowTimestamps", null, (...[, , , value]) =>
+                this.setState({ alwaysShowTimestamps: value as boolean }),
+            ),
+            SettingsStore.watchSetting("showTwelveHourTimestamps", null, (...[, , , value]) =>
+                this.setState({ showTwelveHourTimestamps: value as boolean }),
+            ),
+            SettingsStore.watchSetting("readMarkerInViewThresholdMs", null, (...[, , , value]) =>
+                this.setState({ readMarkerInViewThresholdMs: value as number }),
+            ),
+            SettingsStore.watchSetting("readMarkerOutOfViewThresholdMs", null, (...[, , , value]) =>
+                this.setState({ readMarkerOutOfViewThresholdMs: value as number }),
+            ),
+            SettingsStore.watchSetting("showHiddenEventsInTimeline", null, (...[, , , value]) =>
+                this.setState({ showHiddenEvents: value as boolean }),
+            ),
+            SettingsStore.watchSetting("urlPreviewsEnabled", null, this.onUrlPreviewsEnabledChange),
+            SettingsStore.watchSetting("urlPreviewsEnabled_e2ee", null, this.onUrlPreviewsEnabledChange),
+        ];
+
         this.onRoomViewStoreUpdate(true);
 
         const call = this.getCallForRoom();
@@ -883,7 +883,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
         // stop tracking room changes to format permalinks
         this.stopAllPermalinkCreators();
 
-        dis.unregister(this.dispatcherRef);
+        if (this.dispatcherRef) dis.unregister(this.dispatcherRef);
         if (this.context.client) {
             this.context.client.removeListener(ClientEvent.Room, this.onRoom);
             this.context.client.removeListener(RoomEvent.Timeline, this.onRoomTimeline);
@@ -1106,11 +1106,17 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
     };
 
     private onLocalRoomEvent(roomId: string): void {
-        if (roomId !== this.state.room.roomId) return;
-        createRoomFromLocalRoom(this.context.client, this.state.room as LocalRoom);
+        if (roomId !== this.state.room?.roomId) return;
+        createRoomFromLocalRoom(this.context.client!, this.state.room as LocalRoom);
     }
 
-    private onRoomTimeline = (ev: MatrixEvent, room: Room | null, toStartOfTimeline: boolean, removed, data): void => {
+    private onRoomTimeline = (
+        ev: MatrixEvent,
+        room: Room | undefined,
+        toStartOfTimeline: boolean | undefined,
+        _removed: boolean,
+        data: IRoomTimelineData,
+    ): void => {
         if (this.unmounted) return;
 
         // ignore events for other rooms or the notification timeline set
@@ -1371,7 +1377,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
         this.updateRoomMembers();
     };
 
-    private onMyMembership = (room: Room, membership: string, oldMembership: string): void => {
+    private onMyMembership = (room: Room): void => {
         if (room.roomId === this.state.roomId) {
             this.forceUpdate();
             this.loadMembersIfJoined(room);