mikoskinen / Blazor.EventAggregator

Lightweight Event Aggregator for Blazor (Razor Components).
MIT License
120 stars 23 forks source link

Unsubscribe? #1

Open dradovic opened 5 years ago

dradovic commented 5 years ago

Does the subscriber have to call Unsubscribe in order to prevent memory leaks (as the EventAggregator is registered as a singleton)?

dradovic commented 5 years ago

According to my test, it seems that yes. A component subscribing itself to the event aggregator (as shown in the documentation) should implement IDisposable and then unsubscribe itself. Otherwise, its handler will still be called event if the user has navigated to another page.

dodyg commented 4 years ago

Yeah it needs to unsubscribe at IDisposable.Dispose at each listening component.

ryanbuening commented 3 years ago

@dodyg is there a way to automatically unsubscribe on each listening component? Or is it required to manually put that logic in each component? Is there an example of how you unsubscribe at IDisposable.Dispose for Blazor Server?

dodyg commented 3 years ago

This is the pattern I am using in each of the component that cares

@implements IDisposable
@implements EventAggregator.Blazor.IHandle<JobNominationPagingInfo>
   protected override async Task OnInitializedAsync()
   {
        base.SubscribeEvents();
   }

    public Task HandleAsync(JobNominationPagingInfo pi)
    {
        return Task.CompletedTask;
    }

    public void Dispose()
    {
        base.UnsubscribeEvents();
    }
ryanbuening commented 3 years ago

Thanks. I'm using a code behind file so I think that solution would look something like the following...

public partial class FooPage : IHandle<FooMessage>, IDisposable
{
    protected override void OnInitialized()
    {
        EventAggregator.Subscribe(this);
    }

    public void Dispose()
    {
        EventAggregator.Unsubscribe(this);
    }
}
ryanbuening commented 3 years ago

Is unsubscribing still necessary if EventAggregator is added as a scoped service?

dodyg commented 3 years ago

There is no such thing as Scoped service in Blazor. It's either Singleton or Transient.

dodyg commented 3 years ago

https://docs.microsoft.com/en-us/aspnet/core/blazor/fundamentals/dependency-injection?view=aspnetcore-5.0&pivots=server

ryanbuening commented 3 years ago

Blazor WebAssembly apps don't currently have a concept of DI scopes. Scoped-registered services behave like Singleton services.

Registering a dependency as Scoped in Blazor Server will result in dependencies that live for the duration of the user’s session. Instances of Scoped dependencies will be shared across pages and components for a single user, but not between different users and not across different tabs in the same browser.

It sounds like a scoped service is what I want for my Blazor Server EventAggregator. I don't want a singleton because I don't want a single instance for all users, across all tabs/browsers.

And I don't want transient because it isn't necessary to obtain a new instance of the EventAggregator for each component.

Thoughts?

ryanbuening commented 3 years ago

Maybe the disconnect is you are using WebAssembly and I'm using Server...

dodyg commented 3 years ago

You can use EventAggregrator in scoped in Blazor Server but you still have to subscribe and unsubscribe.

ryanbuening commented 3 years ago

Ok. And I'm able to register it as a scoped service by doing the following:

services.AddScoped<EventAggregator.Blazor.IEventAggregator, EventAggregator.Blazor.EventAggregator>();

but I'm struggling to determine how to set AutoRefresh = true when using the method above. Do you know how I could accomplish that?

dodyg commented 3 years ago

https://github.com/mikoskinen/Blazor.EventAggregator#note-about-auto-refresh

ryanbuening commented 3 years ago

My issue was the order. I initially had:

services.AddScoped<EventAggregator.Blazor.IEventAggregator, EventAggregator.Blazor.EventAggregator>();
services.AddEventAggregator(options => options.AutoRefresh = true);

which caused IEventAggregator to be registered as a singleton which for Blazor Server is not desirable.

Switching to the following seems to be the workaround:

// the order matters!
services.AddEventAggregator(options => options.AutoRefresh = true); // this registers as a singleton
services.AddScoped<EventAggregator.Blazor.IEventAggregator, EventAggregator.Blazor.EventAggregator>(); // now register as scoped

Thanks!