Should IRenderFragment.Dispose also dispose the wrapped component

[linkdotnet]

The current setup if I want to test a component which implements IDisposable/IAsyncDisposable is to go via the instance and call the respective method (Dispose/DisposeAsync).

var cut = RenderComponent<MyComponent>();
cut.Instance.Dispose();

// Assert something
MyComponent.DisposeWasCalled.ShouldBeTrue(); // Let's assume we have a static field which gets set on Dispose

Now what does not work if we Dispose the IRenderFragmentitself:

var cut = RenderComponent<MyComponent>();
cut.Dispose();

MyComponent.DisposeWasCalled.ShouldBeTrue(); // This will fail as our Dispose/DisposeAsync doesn't get called at all

For simpler scenarios like that, there is no issue. The problem arises when you have multiple components which implement IDisposable / IAsyncDisposable:

Parent.razor:

@implements IDisposable

<Child></Child>

@code {
    public void Dispose()
    {
        Console.WriteLIne("Parent was disposed");
    }

Child.razor:

@implements IDisposable

@code {
    public void Dispose()
    {
        Console.WriteLIne("Child was disposed");
    }

If we would navigate away from the site, Blazor would dispose from parent to child, so the console would print something like:

Parent was diposed
Child was disposed

We already have that behavior when an items (plus all its subitems) get removed from the render tree.
Therefore my question: Would this be worth adding as a feature to the IRenderFragment.Dipose method.

[linkdotnet]

If this is wished the simplest solution is to remove the wrapped element in Dispose and let Blazor do the work (which then invokes Dispose/DisposeAsync).

[egil]

Since IRenderedFragmentBase already implements IDisposable, and could be made to implement IAsyncDisposable, its not a bad idea to support triggering the CUT's dispose method, if any, that way.

Without further design/analysis my idea to support this is:

1. All RenderFragments are rendered inside the WrapperComponent in bUnit.
2. If WrapperComponent gets support to remove whatever RenderFragment it has been passed from the render tree again, then the Blazor renderer will take care of any disposing that is needed.
3. So we need a way from the IRenderedFragmentBase to get to the root WrapperComponent and calling a method on it during a call to the IRenderedFragment.Dispose method.
4. After the Dispose call, there should be checks in the code that throws appropriate exceptions indicating that the IRenderedFragmentBase has been disposed and is no longer usable.

[linkdotnet]

Hey Egil, hope you're having a great day.
Thanks for your input on that one.
On top of my head there are two cases which can't be covered by disposing the wrapped component.

The first one I've described on top. A Parent component with Dispose which is owning the Child component with Dispose.
Just calling Dispose on the parent element does not automatically call Dispose on the child.

That said we even have another case, which isn't very uncommon:

Parent.razor:

<Child></Child>

Child.razor:

@implements IDisposable
@code {
    public void Dispose()
    {
        // Do something here
    }
}

When we navigate away from Parent (and therefore remove it from the render tree) the Dispose method from Child gets called anyway.
Now bringing this back to our WrapperComponent: The component does not see the Child which implements IDisposable.

Therefore as a possible solution:
When we call IRenderedFragmentBase.Dispose we detach the RenderFragment given by the user and attach just an empty RenderFragment ourselves. If we now force another re-render on Blazor side, all the necessary actions get invoked.
Very good point about exceptions: I am still thinking of a good design. We don't want to bubble this exception til the user... or at least not unhandled. If we detach the users component and attach our placeholder to force the re-render, do we want to revert that action when an exception happens? Do we want to update the markup to be basically empty?
As a first draft I'd go with that simple solution.

Let me know about your thoughts. If you think this is worth a shot I'd like to do so. Furthermore we can "convert" this into an issue.

Cheers
Steven

EDIT: I didn't read your idea completely and this was exactly my thought as well. I'll just leave my (now obsolete comment) for transparency.

[linkdotnet]

Small amendment: I am still not certain due to the given points on top, if we would need IAsyncDisposable on our IRenderFragmentBase in the first place. Maybe an extension helper like WaitForDispose would be more consistent if we are dealing with IAsyncDisposable On the wrapped component.

[egil]

Hehe not really sure what parts are relevant here :)

[linkdotnet]

Yeah really sorry for that one :D

[linkdotnet]

I guess tomorrow I have a small draft / proposal PR. Have to figure one or two things out where I can’t wrap my head around right now.
Or maybe you can give me one or two Tipps.

[egil]

Actually, there is most likely no need for IAsyncDisposable on IRenderedFragmentBase, since the Blazor renderer will handle the disposing for us. We just need a way to trigger it, and the most natural way to do so is remove the component from the render tree, which we can do via the WrapperComponent, which is ultimately the root for all component render trees created via calls to Render/RenderComponent.

However, this approach only works for IRenderedFragmentBase (and derived) that is returned from TestRenderer.Render (and this TestContext.Render/TestContext.RenderComponent), and NOT via IRenderedFragment.FindComponent /IRenderedFragment.FindComponents methods, as these does not wrap the found components in a WrapperComponent (IIRC). In other words, we wont be able to support:

var cut = Render(@<Parent />);
var child = cut.FindComponent<Child>();
child.Dispose(); // DOES NOT WORK

Since it is the <Parent /> component that controls whether or not the <Child /> component is added to the render tree or not.

As for async disposable components, the existing wait for logic should just work, e.g.:

var cut = Render(@<MyComp/>);

cut.Dispose();

cut.WaitForState(() => cut.HasComponent<MyComp>());

However, calling a method (WaitForState and HasComponent) on a class after its Dispose method has been called, like in this example, seems wrong, so that might be another reason to NOT call that method Dispose.

[linkdotnet]

After a bit of REM sleep I could sort my thoughts a bit :D.

Yes Dispose might be the wrong word. For the PoC I simply call it Detach to stay in the Blazor wording.

We are also calling FindComponents inside RenderInsideRenderTree (inside RenderComponent<TComp> when the component gets really rendered the first time) which, from an user point of view, will gives us already two different IRenderedFragmentBases. The one attached to the WrapperComponent and the one the user retrieves.

[egil]

After a bit of REM sleep I could sort my thoughts a bit :D.

Yes Dispose might be the wrong word. For the PoC I simply call it Detach to stay in the Blazor wording.

I like Detach.

We are also calling FindComponents inside RenderInsideRenderTree (inside RenderComponent<TComp> when the component gets really rendered the first time) which, from an user point of view, will gives us already two different IRenderedFragmentBases. The one attached to the WrapperComponent and the one the user retrieves.

Ahh, you are right, Forgot about that. Looking forward to seeing what you come up with. Feeling a bit under the weather these days, so do not have much energy to deep dive on this.

[linkdotnet]

Hey Egil, first things first take a rest. This has no urgency at all.
I will create two draft Pull Requests... basically we have two ways to tackle that problem depending how deep we want to go.
I'll explain in both PR's the pros and cons.

Here the PR's in question:
Based on TestContext: #645
Based on RenderFragment: #646

Disclaimer: The next part most likely will frame you :D

I tried both and to be honest the approach with TestContext is more reliable and should fit most of the needs.
I just took another approach just to check how far I can go to detach individual components inside the render-tree. But as you might see, there are lot of open questions.