Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

MidhunSureshR
Copy link
Member

Builds on from #30398

Previously, SubscriptionViewModel had the following mechanism for adding/removing any event listeners that the vm needs:

This means that the vm only listens to any relevant events (that it needs to keep its state correct) as long as there is at least one view subscribed to it. This is a problem because if the view unmounts and then mounts again, meaning that it goes through subscribe -> unsubscribe -> subscribe, the view model can present the wrong state because it wasn't listening to relevant events. Therefore, the lifetime of the event listeners should equal the lifetime of the view model itself.
Issue is illustrated here: https://github.com/element-hq/element-web/blob/c64104ffeea69da579ad601f9a4e0af8624e05a9/src/shared-components/ViewModelSubIssueDemo.test.tsx

This PR borrows the concept of Disposables from hydrogen:
It provides a way to track any resource that needs to be relinquished eventually. This will usually be sub view models or event listeners.
Whoever creates the view model has the responsibility to call dispose() method on it to indicate that it is no longer needed.

Tracking sub view-models

class FooViewModel extends BaseViewModel {
    private barViewModel: BarViewModel;
    constructor() {
        // barViewModel will be disposed whenever this vm is disposed
        this.barViewModel = this.disposables.track(new BarViewModel());
    }
}

Tracking event listeners

class FooViewModel extends BaseViewModel {
    constructor() {
        // This listener will be removed whenever this vm is disposed
        this.disposables.trackListener(props.someEmitter, "SOME_EVENT", () => { this.doSomething(); });
    }
   
   doSomething() {
      // ...
   }
}

@MidhunSureshR MidhunSureshR added the T-Task Tasks for the team like planning label Aug 5, 2025
@MidhunSureshR MidhunSureshR changed the base branch from develop to midhun/mvvm/state-management-1 August 5, 2025 08:45
Base automatically changed from midhun/mvvm/state-management-1 to develop August 6, 2025 12:46
Copy link
Member

@florianduros florianduros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the dispose function is never called for the two vms (AudioPlayer & TextuamEvent). For AudioPlayer, you can call in the unmount of the MAudioBody

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants