Aspnetcore: Please add a Destroyed event to IComponent (Possible critical WASM/Mono bug)

Created on 21 Jul 2018  路  9Comments  路  Source: dotnet/aspnetcore

I am trying to add a state subscription to my Fluxor library. To subscribe you'd do this

@inject IState<AppState> AppState

@functions {
  AppState.Subscribe(this);
}

Fluxor will then call back the StateHasChanged of that instance every time the state changes, but I need to also know when the component is destroyed so I can automatically unsubscribe.

It would be good also if I could call StateHasChanged without having to use reflection.

area-blazor

Most helpful comment

@mrpmorris The design intent is that component authors get to control the encapsulation. If they want to expose some notification when there's disposal they can do. If they want to expose a public method for triggering StateHasChanged they can do. But if they don't choose to do so, the default encapsulation doesn't expose this. Like C# classes in general, encapsulation is preferred by default, allowing developers to choose what to open up.

I don't want to have to force people to descend their components from a specific component because that will prevent them from using another library that also forces them to do the same.

This is a very good and important point. What would be ideal would be having both a Fluxor-specific base class people can use right out of the box if they want, plus a Fluxor state management interface that other developers can implement if they want to mix in that logic with some unrelated base class.

In general, Blazor-related frameworks are going to want to hook into components in various ways. We could ignore encapsulation and try to open up every possible lifecycle step for hooking into and modifying the behavior dynamically at runtime. But other frameworks that have taken this philosophy have run aground quickly. It would make it impossible to evolve how Blazor works over time, because every aspect of component lifecycle would be unchangeable without breaking people. And of course it would mean that nobody can really have much encapsulation for their components. So rather than doing that, my hope is that we can use C# features such as interfaces and inheritance to give people composable ways of extending and integrating.

One other thing that might help is if we supported some way of declaring a project-level "default Blazor component base class". That is, if developers could configure that all their components should inherit from FluxorComponentBase unless otherwise specified, it makes it pretty easy for people to use Fluxor in typical cases. In the more advanced case where a developer wants to combine multiple third-party integrations, they'd need to inherit from a custom base class that implements the interfaces from all those third-party integrations.

Does that make sense? I hope so. I'm going to mark this closed because we're still hoping the current mechanism will suffice. But please let us know what you think.

cc @danroth27

All 9 comments

The following works:

@page "/demo"
@implements IDisposable

@functions {
    public void Dispose()
    {
        Console.WriteLine("Disposed");
    }
}

@Suchiman That's in the component itself, I need a 3rd party class instance to know when it happens. I would like to replace lines 81+ so that I don't have to use weak references, garbage collection, or reflection.

https://github.com/mrpmorris/blazor-fluxor/blob/master/src/Blazor.Fluxor/Feature.cs

Then raise an event instead of calling Console.WriteLine?

That doesn't mean you have to implement it everywhere, you can just use

public class DisposableComponent : BlazorComponent, IDisposable
{
    public event EventHandler<EventArgs> OnDestroy;

    public void Dispose() => OnDestroy?.Invoke(this, EventArgs.Empty);
}
@page "/demo"
@inherits DisposableComponent

@Suchiman I am writing a library. I don't want to have to force people to descend their components from a specific component because that will prevent them from using another library that also forces them to do the same.

I can currently wait for the reference to the component to be collected by the garbage collector, but it will probably have been removed from the view tree before that. I would just like to be able to detect when it is removed from the tree (destroyed) so I can stop sending it notifications. It would also be very useful if I could execute it's StateHasChanged method without having to use reflection too.

@mrpmorris The design intent is that component authors get to control the encapsulation. If they want to expose some notification when there's disposal they can do. If they want to expose a public method for triggering StateHasChanged they can do. But if they don't choose to do so, the default encapsulation doesn't expose this. Like C# classes in general, encapsulation is preferred by default, allowing developers to choose what to open up.

I don't want to have to force people to descend their components from a specific component because that will prevent them from using another library that also forces them to do the same.

This is a very good and important point. What would be ideal would be having both a Fluxor-specific base class people can use right out of the box if they want, plus a Fluxor state management interface that other developers can implement if they want to mix in that logic with some unrelated base class.

In general, Blazor-related frameworks are going to want to hook into components in various ways. We could ignore encapsulation and try to open up every possible lifecycle step for hooking into and modifying the behavior dynamically at runtime. But other frameworks that have taken this philosophy have run aground quickly. It would make it impossible to evolve how Blazor works over time, because every aspect of component lifecycle would be unchangeable without breaking people. And of course it would mean that nobody can really have much encapsulation for their components. So rather than doing that, my hope is that we can use C# features such as interfaces and inheritance to give people composable ways of extending and integrating.

One other thing that might help is if we supported some way of declaring a project-level "default Blazor component base class". That is, if developers could configure that all their components should inherit from FluxorComponentBase unless otherwise specified, it makes it pretty easy for people to use Fluxor in typical cases. In the more advanced case where a developer wants to combine multiple third-party integrations, they'd need to inherit from a custom base class that implements the interfaces from all those third-party integrations.

Does that make sense? I hope so. I'm going to mark this closed because we're still hoping the current mechanism will suffice. But please let us know what you think.

cc @danroth27

https://github.com/mrpmorris/blazor-fluxor/blob/master/src/Blazor.Fluxor/Feature.cs

I think changing the method Subscribe (line 81) so that if the passed instance also implements IFluxorComponent then it should add a callback to a IFluxorComponent.Disposed event will do the trick, thanks for the suggestion! I assume calling StateHasChanged on a BlazorComponent that has been removed will not do any harm? (It seems not to so far).

I would also like to change the BlazorComponent parameter type to IComponent but I cannot guarantee a StateHasChanged method will exist, as IComponent doesn't have it. Do you have any suggestions for this?

I assume calling StateHasChanged on a BlazorComponent that has been removed will not do any harm?

Correct, it's fine.

I cannot guarantee a StateHasChanged method will exist, as IComponent doesn't have it. Do you have any suggestions for this?

In general a Blazor component doesn't have to have a StateHasChanged method. That's an artifact of the BlazorComponent base class. You're probably best off sticking with BlazorComponent rather than IComponent if you require that to exist.

Thanks!

Was this page helpful?
0 / 5 - 0 ratings