Add a Disposed event to BaseComponent

[mrpmorris]

Is your feature request related to a problem? Please describe.

I am trying to add Reactive patterns to Blazor-Fluxor, in order to compete with Angular's ngrx-Store. Angular uses a lot of boilerplate that I would like to avoid in Blazor.

this.SomeInjectedFeatureState .TakeUntilDisposed(this) // Would require a public Disposed event .NotifyStateHasChanged(this) // Would be better if StateHasChanged were public .Subscribe();

Describe the solution you'd like

Add an event to BaseComponent that is triggered when it is disposed. - So my library can automatically unsubscribe from the Rx Stream and avoid memory leaks.

Describe alternatives you've considered

Without the Disposed event the user of my library must do the following in their components 1: Create a Reactive Subject to notify RX when this component has been disposed. 2: Override Dispose to Complete the above subject when subscribing

this.SomeInjectedState.TakeUntil(HasBeenDisposed).Subscribe(x => StateHasChanged());

Steps 1 and 2 are the pain point when writing Angular apps. In Angular apps it would look like this

private ngUnsubscribe: Subject = new Subject();

ngOnInit() { this.store.select(selectors.selectCustomer) .pipe(takeUntil(this.ngUnsubscribe)) .subscribe((customer) = > { this.customerIsCool = customer.supports === 'Southend'; }); });

ngOnDestroy() { this.ngUnsubscribe.next(); this.ngUnsubscribe.complete(); }

If you compare that to my desired implementation, it is nowhere near as concise and is a lot of work.

this.SomeInjectedFeatureState.TakeUntilDisposed(this).NotifyStateHasChanged(this).Subscribe();

Additional context

https://medium.com/@stodge/ngrx-common-gotchas-8f59f541e47c

[mkArtakMSFT]

Thanks for contacting us, @mrpmorris. @SteveSandersonMS what do you think about this ask?

[SteveSandersonMS]

StateHasChanged is protected because that lets each component make its own choice about whether to expose it publicly, and hence whether it manages its own re-rendering decisions or not. We don't want to force every component to have a public StateHasChanged by default because that breaks encapsulation.

If you want all your components to have a public version of that, create a base class with a public method that itself calls StateHasChanged.

I understand your scenario in particular is writing library code that you want to work with other people's components. In general we don't tend to solve such problems by making more things public, but rather by making it easy for developers to opt into the requirements of your library. For example you could:

• Offer a base class people can inherit from
• Or offer an interface INotifyRefresh that people can implement (example: void INotifyRefresh.Refresh() => StateHasChanged();

Either of these would be more correct than using reflection to call StateHasChanged, because Blazor components don't even have to have a StateHasChanged method at all. For components that implement IComponent directly and don't inherit from ComponentBase, there is no such method.

[mrpmorris]

@SteveSandersonMS The StateHasChanged part was marked optional, the majority of the ticket was about implementing a Disposed event on BaseComponent.

This wouldn't break encapsulation, and would help 3rd party frameworks such as mine avoid leaking memory. I have altered the ticket accordingly.

Could this ticket be re-opened for consideration please? @mkArtakMSFT

[SteveSandersonMS]

Thanks for explaining. I’ve reopened.

In the short term a base class is still the most comprehensive way to support this, but I appreciate your goal of not forcing that. There are multiple ways we could enable this for frameworks, so the chosen design will depend on what other scenarios emerge after 3.0 ships.

[mrpmorris]

This can be closed, because it is now possible to implement IDisposable within a Razor file.

[mrpmorris]

I'd like to reopen this because an OnDisposed event (not EventCallback) would be really useful for component library authors - and having the user implement IDisposable would both be more work + wouldn't actually solve the problem I am trying to solve in this request.

[SteveSandersonMS]

It sounds like your library has a reference to some other component instances. Is that right? Could you briefly summarise how the API for you library looks? I'm unsure what the scenario really looks like, because I don't know what useful thing you would be able to do with a reference to another component instance anyway.

[mrpmorris]

Hi @SteveSandersonMS

I've not written it yet. The idea is to create a reactive library for Blazor. Subscriptions need to be manually unsubscribed, so if I create an extension that takes a ComponentBase I can subscribe to an OnDisposed event from there automatically cancel the subscription. It'd need to be an event type that can accept multiple observers (standard .net event I expect).

As the source observable will have a long lifetime, if the subscription isn't cancelled it will hold onto a reference to the disposed component.

[SteveSandersonMS]

OK, well please let us know when you have a design in mind for your library, and then we can evaluate how much we think the requirements for this kind of API will likely generalize to other use cases and libraries too.

Currently I'm unsure how good an API you can build using an extension method on ComponentBase, considering how even calling it from itself would look really weird (you'd need this.RegisterSomething() instead of just RegisterSomething(), which would massively confuse people). It might be that you end up finding a better design is to have something like a <RegisterSomething /> component or a different design altogether. As such we need to wait and see before making decisions about what we'd do in the core libraries. Naturally, additions to the core libraries need to generalise a lot, more than just meeting the needs of one other library.

[mrpmorris]

Here is an example. 1: I have a service in my app that uses the Microsoft Reactive library

public interface IEventNotificationService
{
    IObservable<object> Events { get; }
    void Publish(object @event);
}
public class EventNotificationService : IEventNotificationService
{
    private Subject<object> Subject = new Subject<object>();
    public IObservable<object> Events => Subject;

    public void Publish(object @event)
    {
        Subject.OnNext(@event);
    }
}

If ComponentBase had a Disposed event then I could write this extension class.

public static class ObservableExtensions
{
    public static IObservable<T> TakeUntilDisposed<T>(this IObservable<T> observable, ComponentBase2 component)
    {
        var subject = new Subject<object>();
        component.Disposed += (s, e) =>
        {
            subject.OnNext(null);
            subject.OnCompleted();
            subject.Dispose();
        };
        return observable.TakeUntil(subject);
    }
}

Ultimately, consumers of the events could prevent memory leaks by using TakeUntilDisposed, like this

protected override void OnInitialized()
{
    base.OnInitialized();
    EventNotificationService.Events.TakeUntilDisposed(this).OfType<int>().Subscribe(x =>
    {
        Counter = x;
        InvokeAsync(StateHasChanged);
    });
}

The goal is for this to work with any ComponentBase, so that consumers of my library do not have to descend their components from a specific base class.

@SteveSandersonMS I hope this is sufficient detail? I believe this could be a very useful hook point for library developers.

[javiercn]

We'll consider this feature during the next release planning period and update the status of this issue accordingly.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.