Compare commits

...

13 Commits

Author SHA1 Message Date
R Midhun Suresh
a76654ff22 Fix license 2025-08-23 20:14:01 +05:30
R Midhun Suresh
a0a03148e4 Use hook in TextualEventFactory 2025-08-23 20:14:01 +05:30
R Midhun Suresh
14af817037 Add hook to auto dispose viewmodels 2025-08-23 20:13:58 +05:30
R Midhun Suresh
1e449ae215 Merge branch 'develop' into midhun/mvvm/state-management-2 2025-08-23 20:10:51 +05:30
R Midhun Suresh
c08e756854 Dispose AudioPlayerViewModel 2025-08-11 15:09:20 +05:30
R Midhun Suresh
1927a4b235 Expose isDisposed through base vm 2025-08-11 11:03:40 +05:30
R Midhun Suresh
c3f1879189 Fix audio player vm 2025-08-11 11:01:32 +05:30
R Midhun Suresh
ce2f9ae32a Throw error in trackListener as well 2025-08-11 10:43:36 +05:30
R Midhun Suresh
4caf52abf3 No-op on dispose call instead of throwing error 2025-08-11 10:42:44 +05:30
R Midhun Suresh
d86dcb1b7b Update vm so that the listener is tracked through disposable 2025-08-11 10:34:13 +05:30
R Midhun Suresh
2cd2a4a8ef Use disposable in BaseViewModel 2025-08-11 10:34:12 +05:30
R Midhun Suresh
3bbb62d346 Remove old code 2025-08-11 10:34:12 +05:30
R Midhun Suresh
4e42654c4f Introduce disposables to track sub vms and event listeners 2025-08-11 10:34:09 +05:30
10 changed files with 267 additions and 67 deletions

View File

@@ -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 {

View File

@@ -45,6 +45,7 @@ import ModuleApi from "../modules/Api";
import { TextualEventViewModel } from "../viewmodels/event-tiles/TextualEventViewModel";
import { TextualEventView } from "../shared-components/event-tiles/TextualEventView";
import { ElementCallEventType } from "../call-types";
import { useAutoDisposedViewModel } from "../viewmodels/base/useAutoDisposedViewModel";
// Subset of EventTile's IProps plus some mixins
export interface EventTileTypeProps
@@ -79,10 +80,15 @@ const LegacyCallEventFactory: Factory<FactoryProps & { callEventGrouper: LegacyC
<LegacyCallEvent ref={ref} {...props} />
);
const CallEventFactory: Factory = (ref, props) => <CallEvent ref={ref} {...props} />;
export const TextualEventFactory: Factory = (ref, props) => {
const vm = new TextualEventViewModel(props);
const TextualEventComponent: React.FC<FactoryProps> = (props) => {
const vm = useAutoDisposedViewModel(() => new TextualEventViewModel(props));
return <TextualEventView vm={vm} />;
};
export const TextualEventFactory: Factory = (ref, props) => {
return <TextualEventComponent {...props} />;
};
const VerificationReqFactory: Factory = (_ref, props) => <MKeyVerificationRequest {...props} />;
const HiddenEventFactory: Factory = (ref, props) => <HiddenBody ref={ref} {...props} />;

View File

@@ -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.
*/

View File

@@ -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;
}
}

View File

@@ -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;
}
}

View File

@@ -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();
}
};
};

View File

@@ -0,0 +1,62 @@
/*
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 { useEffect, useState } from "react";
import type { BaseViewModel } from "./BaseViewModel";
type VmCreator<B extends BaseViewModel<unknown, unknown>> = () => B;
/**
* Instantiate a view-model that gets disposed when the calling react component unmounts.
* In other words, this hook ties the lifecycle of a view-model to the lifecycle of a
* react component.
*
* @param vmCreator A function that returns a view-model instance
* @returns view-model instance from vmCreator
* @example
* const vm = useAutoDisposedViewModel(() => new FooViewModel({prop1, prop2, ...});
*/
export function useAutoDisposedViewModel<B extends BaseViewModel<unknown, unknown>>(vmCreator: VmCreator<B>): B {
/**
* The view-model instance may be replaced by a different instance in some scenarios.
* We want to be sure that whatever react component called this hook gets re-rendered
* when this happens, hence the state.
*/
const [viewModel, setViewModel] = useState<B>(vmCreator);
/**
* Our intention here is to ensure that the dispose method of the view-model gets called
* when the component that uses this hook unmounts.
* We can do that by combining a useEffect cleanup with an empty dependency array.
*/
useEffect(() => {
let toDispose = viewModel;
/**
* Because we use react strict mode, react will run our effects twice in dev mode to make
* sure that they are pure.
* This presents a complication - the vm instance that we created in our state initializer
* will get disposed on the first cleanup.
* So we'll recreate the view-model if it's already disposed.
*/
if (viewModel.isDisposed) {
const newViewModel = vmCreator();
// Change toDispose so that we don't end up disposing the already disposed vm.
toDispose = newViewModel;
setViewModel(newViewModel);
}
return () => {
// Dispose the view-model when this component unmounts
toDispose.dispose();
};
// eslint-disable-next-line react-compiler/react-compiler
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
return viewModel;
}

View File

@@ -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);
};
}

View File

@@ -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);
});
});

View File

@@ -0,0 +1,47 @@
/*
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 { renderHook } from "jest-matrix-react";
import { BaseViewModel } from "../../../src/viewmodels/base/BaseViewModel";
import { useAutoDisposedViewModel } from "../../../src/viewmodels/base/useAutoDisposedViewModel";
class TestViewModel extends BaseViewModel<{ count: number }, { initial: number }> {
constructor(props: { initial: number }) {
super(props, { count: props.initial });
}
public increment() {
const newCount = this.getSnapshot().count + 1;
this.snapshot.set({ count: newCount });
}
}
describe("useAutoDisposedViewModel", () => {
it("should return view-model", () => {
const vmCreator = () => new TestViewModel({ initial: 0 });
const { result } = renderHook(() => useAutoDisposedViewModel(vmCreator));
const vm = result.current;
expect(vm).toBeInstanceOf(TestViewModel);
expect(vm.isDisposed).toStrictEqual(false);
});
it("should dispose view-model on unmount", () => {
const vmCreator = () => new TestViewModel({ initial: 0 });
const { result, unmount } = renderHook(() => useAutoDisposedViewModel(vmCreator));
const vm = result.current;
vm.increment();
unmount();
expect(vm.isDisposed).toStrictEqual(true);
});
it("should recreate view-model on react strict mode", async () => {
const vmCreator = () => new TestViewModel({ initial: 0 });
const output = renderHook(() => useAutoDisposedViewModel(vmCreator), { reactStrictMode: true });
const vm = output.result.current;
expect(vm.isDisposed).toStrictEqual(false);
});
});