MVVM - Introduce the concept of disposables to track event listeners, sub vms and so on (#30475)

* Introduce disposables to track sub vms and event listeners

* Remove old code

* Use disposable in BaseViewModel

* Update vm so that the listener is tracked through disposable

* No-op on dispose call instead of throwing error

* Throw error in trackListener as well

* Fix audio player vm

* Expose isDisposed through base vm

* Dispose AudioPlayerViewModel

src/components/views/messages/MAudioBody.tsx

@@ -74,6 +74,7 @@ export default class MAudioBody extends React.PureComponent<IBodyProps, IState>
 
     public componentWillUnmount(): void {
         this.state.playback?.destroy();
+        this.state.audioPlayerVm?.dispose();
     }
 
     protected get showFileBody(): boolean {

src/viewmodels/audio/AudioPlayerViewModel.ts

@@ -77,6 +77,9 @@ export class AudioPlayerViewModel
 
     public constructor(props: Props) {
         super(props, AudioPlayerViewModel.computeSnapshot(props.playback, props.mediaName));
+        this.disposables.trackListener(props.playback, UPDATE_EVENT, this.setSnapshot);
+        // There is no unsubscribe method in SimpleObservable
+        this.props.playback.clockInfo.liveData.onUpdate(this.setSnapshot);
 
         // Don't wait for the promise to complete - it will emit a progress update when it
         // is done, and it's not meant to take long anyhow.
@@ -97,15 +100,6 @@ export class AudioPlayerViewModel
         }
     }
 
-    protected addDownstreamSubscription(): void {
-        this.props.playback.on(UPDATE_EVENT, this.setSnapshot);
-        // There is no unsubscribe method in SimpleObservable
-        this.props.playback.clockInfo.liveData.onUpdate(this.setSnapshot);
-    }
-    protected removeDownstreamSubscription(): void {
-        this.props.playback.off(UPDATE_EVENT, this.setSnapshot);
-    }
-
     /**
      * Sets the snapshot and emits an update to subscribers.
      */

src/viewmodels/base/BaseViewModel.ts

@@ -6,6 +6,7 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import { type ViewModel } from "../../shared-components/ViewModel";
+import { Disposables } from "./Disposables";
 import { Snapshot } from "./Snapshot";
 import { ViewModelSubscriptions } from "./ViewModelSubscriptions";
 
@@ -13,13 +14,11 @@ export abstract class BaseViewModel<T, P> implements ViewModel<T> {
     protected subs: ViewModelSubscriptions;
     protected snapshot: Snapshot<T>;
     protected props: P;
+    protected disposables = new Disposables();
 
     protected constructor(props: P, initialSnapshot: T) {
         this.props = props;
-        this.subs = new ViewModelSubscriptions(
-            this.addDownstreamSubscriptionWrapper,
-            this.removeDownstreamSubscriptionWrapper,
-        );
+        this.subs = new ViewModelSubscriptions();
         this.snapshot = new Snapshot(initialSnapshot, () => {
             this.subs.emit();
         });
@@ -29,37 +28,24 @@ export abstract class BaseViewModel<T, P> implements ViewModel<T> {
         return this.subs.add(listener);
     };
 
-    /**
-     * Wrapper around the abstract subscribe callback as we can't assume that the subclassed method
-     * has a bound `this` context.
-     */
-    private addDownstreamSubscriptionWrapper = (): void => {
-        this.addDownstreamSubscription();
-    };
-
-    /**
-     * Wrapper around the abstract unsubscribe callback as we can't call pass an abstract method directly
-     * in the constructor.
-     */
-    private removeDownstreamSubscriptionWrapper = (): void => {
-        this.removeDownstreamSubscription();
-    };
-
-    /**
-     * Called when the first listener subscribes: the subclass should set up any necessary subscriptions
-     * to call this.subs.emit() when the snapshot changes.
-     */
-    protected abstract addDownstreamSubscription(): void;
-
-    /**
-     * Called when the last listener unsubscribes: the subclass should clean up any subscriptions.
-     */
-    protected abstract removeDownstreamSubscription(): void;
-
     /**
      * Returns the current snapshot of the view model.
      */
     public getSnapshot = (): T => {
         return this.snapshot.current;
     };
+
+    /**
+     * Relinquish any resources held by this view-model.
+     */
+    public dispose(): void {
+        this.disposables.dispose();
+    }
+
+    /**
+     * Whether this view-model has been disposed.
+     */
+    public get isDisposed(): boolean {
+        return this.disposables.isDisposed;
+    }
 }

src/viewmodels/base/Disposables.ts

@@ -0,0 +1,70 @@
+/*
+Copyright 2025 New Vector Ltd.
+
+SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE files in the repository root for full details.
+*/
+
+import type { EventEmitter } from "events";
+
+/**
+ * Something that needs to be eventually disposed. This can be:
+ * - A function that does the disposing
+ * - An object containing a dispose method which does the disposing
+ */
+export type DisposableItem = { dispose: () => void } | (() => void);
+
+/**
+ * This class provides a way for the view-model to track any resource
+ * that it needs to eventually relinquish.
+ */
+export class Disposables {
+    private readonly disposables: DisposableItem[] = [];
+    private _isDisposed: boolean = false;
+
+    /**
+     * Relinquish all tracked disposable values
+     */
+    public dispose(): void {
+        if (this.isDisposed) return;
+        this._isDisposed = true;
+        for (const disposable of this.disposables) {
+            if (typeof disposable === "function") {
+                disposable();
+            } else {
+                disposable.dispose();
+            }
+        }
+    }
+
+    /**
+     * Track a value that needs to be eventually relinquished
+     */
+    public track<T extends DisposableItem>(disposable: T): T {
+        this.throwIfDisposed();
+        this.disposables.push(disposable);
+        return disposable;
+    }
+
+    /**
+     * Add an event listener that will be removed on dispose
+     */
+    public trackListener(emitter: EventEmitter, event: string, callback: (...args: unknown[]) => void): void {
+        this.throwIfDisposed();
+        emitter.on(event, callback);
+        this.track(() => {
+            emitter.off(event, callback);
+        });
+    }
+
+    private throwIfDisposed(): void {
+        if (this.isDisposed) throw new Error("Disposable is already disposed");
+    }
+
+    /**
+     * Whether this disposable has been disposed
+     */
+    public get isDisposed(): boolean {
+        return this._isDisposed;
+    }
+}

src/viewmodels/base/ViewModelSubscriptions.ts

@@ -6,20 +6,11 @@ Please see LICENSE files in the repository root for full details.
 */
 
 /**
- * Utility class for view models to manage suscriptions to their updates
+ * Utility class for view models to manage subscriptions to their updates
  */
 export class ViewModelSubscriptions {
     private listeners = new Set<() => void>();
 
-    /**
-     * @param subscribeCallback Called when the first listener subscribes.
-     * @param unsubscribeCallback Called when the last listener unsubscribes.
-     */
-    public constructor(
-        private subscribeCallback: () => void,
-        private unsubscribeCallback: () => void,
-    ) {}
-
     /**
      * Subscribe to changes in the view model.
      * @param listener Will be called whenever the snapshot changes.
@@ -27,15 +18,8 @@ export class ViewModelSubscriptions {
      */
     public add = (listener: () => void): (() => void) => {
         this.listeners.add(listener);
-        if (this.listeners.size === 1) {
-            this.subscribeCallback();
-        }
-
         return () => {
             this.listeners.delete(listener);
-            if (this.listeners.size === 0) {
-                this.unsubscribeCallback();
-            }
         };
     };
 

src/viewmodels/event-tiles/TextualEventViewModel.ts

@@ -17,18 +17,11 @@ export class TextualEventViewModel extends BaseViewModel<TextualEventViewSnapsho
     public constructor(props: EventTileTypeProps) {
         super(props, { content: "" });
         this.setTextFromEvent();
+        this.disposables.trackListener(this.props.mxEvent, MatrixEventEvent.SentinelUpdated, this.setTextFromEvent);
     }
 
     private setTextFromEvent = (): void => {
         const content = textForEvent(this.props.mxEvent, MatrixClientPeg.safeGet(), true, this.props.showHiddenEvents);
         this.snapshot.set({ content });
     };
-
-    protected addDownstreamSubscription = (): void => {
-        this.props.mxEvent.on(MatrixEventEvent.SentinelUpdated, this.setTextFromEvent);
-    };
-
-    protected removeDownstreamSubscription = (): void => {
-        this.props.mxEvent.off(MatrixEventEvent.SentinelUpdated, this.setTextFromEvent);
-    };
 }

test/viewmodels/base/Disposables-test.ts

@@ -0,0 +1,57 @@
+/*
+Copyright 2025 New Vector Ltd.
+SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+Please see LICENSE files in the repository root for full details.
+*/
+
+import { EventEmitter } from "events";
+
+import { Disposables } from "../../../src/viewmodels/base/Disposables";
+
+describe("Disposable", () => {
+    it("isDisposed is true after dispose() is called", () => {
+        const disposables = new Disposables();
+        expect(disposables.isDisposed).toEqual(false);
+        disposables.dispose();
+        expect(disposables.isDisposed).toEqual(true);
+    });
+
+    it("dispose() calls the correct disposing function", () => {
+        const disposables = new Disposables();
+
+        const item1 = {
+            foo: 5,
+            dispose: jest.fn(),
+        };
+        disposables.track(item1);
+
+        const item2 = jest.fn();
+        disposables.track(item2);
+
+        disposables.dispose();
+
+        expect(item1.dispose).toHaveBeenCalledTimes(1);
+        expect(item2).toHaveBeenCalledTimes(1);
+    });
+
+    it("Throws error if acting on already disposed disposables", () => {
+        const disposables = new Disposables();
+        disposables.dispose();
+        expect(() => {
+            disposables.track(jest.fn);
+        }).toThrow();
+    });
+
+    it("Removes tracked event listeners on dispose", () => {
+        const disposables = new Disposables();
+        const emitter = new EventEmitter();
+
+        const fn = jest.fn();
+        disposables.trackListener(emitter, "FooEvent", fn);
+        emitter.emit("FooEvent");
+        expect(fn).toHaveBeenCalled();
+
+        disposables.dispose();
+        expect(emitter.listenerCount("FooEvent", fn)).toEqual(0);
+    });
+});