bUnit-dev / bUnit

bUnit is a testing library for Blazor components that make tests look, feel, and runs like regular unit tests. bUnit makes it easy to render and control a component under test’s life-cycle, pass parameter and inject services into it, trigger event handlers, and verify the rendered markup from the component using a built-in semantic HTML comparer.
https://bunit.dev
MIT License
1.14k stars 105 forks source link

RenderComponent throws unexpected error for component rendered using RenderTreeBuilder #768

Closed jimitndiaye closed 2 years ago

jimitndiaye commented 2 years ago

Describe the bug When rendering a component via bUnit I get an error but when rendering in Blazor (both Server and WASM) the same component works fine.

Example: A stripped down (only dependency is MudBlazor) version of the component is defined below:

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Components;
using Microsoft.AspNetCore.Components.Rendering;
using Microsoft.AspNetCore.Components.Web;
using MudBlazor;

namespace MyProject.ComponentRenderer;

/// <summary>
/// A component that renders another component dynamically according to its
/// <see cref="Type" /> parameter.
/// </summary>
public class DynamicComponentWithErrorBoundary : IComponent
{
    private RenderHandle _renderHandle;
    private readonly RenderFragment _cachedRenderFragment;

    /// <summary>
    /// Constructs an instance of <see cref="DynamicComponentWithErrorBoundary"/>.
    /// </summary>
    public DynamicComponentWithErrorBoundary()
    {
        _cachedRenderFragment = Render;
    }

    /// <summary>
    /// Gets or sets the type of the component to be rendered. The supplied type must
    /// implement <see cref="IComponent"/>.
    /// </summary>
    [Parameter]
    [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)]
    [EditorRequired]
    public Type Type { get; set; } = default!;

    /// <summary>
    /// Gets or sets a dictionary of parameters to be passed to the component.
    /// </summary>
    // Note that this deliberately does *not* use CaptureUnmatchedValues. Reasons:
    // [1] The primary scenario for DynamicComponent is where the call site doesn't
    //     know which child component it's rendering, so it typically won't know what
    //     set of parameters to pass either, hence the developer most likely wants to
    //     pass a dictionary rather than having a fixed set of parameter names in markup.
    // [2] If we did have CaptureUnmatchedValues here, then it would become a breaking
    //     change to ever add more parameters to DynamicComponent itself in the future,
    //     because they would shadow any coincidentally same-named ones on the target
    //     component. This could lead to application bugs.
    [Parameter]
    public IDictionary<string, object>? Parameters { get; set; }

    /// <summary>
    /// Gets rendered component instance.
    /// </summary>
    public object? Instance { get; private set; }

    /// <inheritdoc />
    public void Attach(RenderHandle renderHandle)
    {
        _renderHandle = renderHandle;
    }

    /// <inheritdoc />
    public Task SetParametersAsync(ParameterView parameters)
    {
        // This manual parameter assignment logic will be marginally faster than calling
        // ComponentProperties.SetProperties.
        foreach (var entry in parameters)
        {
            if (entry.Name.Equals(nameof(Type), StringComparison.OrdinalIgnoreCase))
            {
                Type = (Type)entry.Value;
            }
            else if (entry.Name.Equals(nameof(Parameters), StringComparison.OrdinalIgnoreCase))
            {
                Parameters = (IDictionary<string, object>)entry.Value;
            }
            else
            {
                throw new InvalidOperationException(
                    $"{nameof(DynamicComponentWithErrorBoundary)} does not accept a parameter with the name '{entry.Name}'. To pass parameters to the dynamically-rendered component, use the '{nameof(Parameters)}' parameter.");
            }
        }

        if (Type is null)
        {
            throw new InvalidOperationException($"{nameof(DynamicComponentWithErrorBoundary)} requires a non-null value for the parameter {nameof(Type)}.");
        }

        _renderHandle.Render(_cachedRenderFragment);
        return Task.CompletedTask;
    }

    void Render(RenderTreeBuilder builder)
    {
        builder.OpenComponent<ErrorBoundary>(0);
        builder.AddAttribute(1, nameof(ErrorBoundary.ChildContent), (RenderFragment)(childContentBuilder =>
        {
            childContentBuilder.OpenComponent(2, Type);
            if (Parameters is not null)
                foreach (var entry in Parameters)
                    builder.AddAttribute(3, entry.Key, entry.Value);
            childContentBuilder.AddComponentReferenceCapture(4, component =>
            {
                Instance = component;
            });

            childContentBuilder.CloseComponent();
        }
            ));
        builder.AddAttribute(5, nameof(ErrorBoundary.ErrorContent), (RenderFragment<Exception>)(context =>
                errorContentBuilder =>
                {
                    errorContentBuilder.OpenComponent<MudStack>(6);
                    errorContentBuilder.AddAttribute(7, nameof(MudStack.Class), "border-2 mud-border-error");
                    errorContentBuilder.AddAttribute(8, nameof(MudStack.AlignItems), AlignItems.Center);
                    errorContentBuilder.AddAttribute(9, nameof(MudStack.Justify), Justify.Center);
                    errorContentBuilder.AddAttribute(10, nameof(MudStack.Style), "min-height: 50px");
                    errorContentBuilder.AddAttribute(11, nameof(MudStack.ChildContent),
                        (RenderFragment)(alertBuilder =>
                        {
                            alertBuilder.OpenComponent<MudAlert>(12);
                            alertBuilder.AddAttribute(13, nameof(MudAlert.Severity), Severity.Error);
                            alertBuilder.AddAttribute(14, nameof(MudAlert.ChildContent),
                                (RenderFragment)(alertContent =>
                                {
                                    alertContent.AddContent(15,
                                            "An internal UI error occurred. Please contact support.");
                                }
                                ));
                            alertBuilder.CloseComponent();
                        }
                        ));
                    errorContentBuilder.CloseComponent();
                }
            ));
        builder.CloseComponent();
    }
}

With this test:

    [Fact]
    public void ComponentRenderer_WrapsRenderedComponentInErrorBoundary2()
    {
        const string text = "this is a test";
       // this lines throws the error (if the Parameters parameter is added.
        var cut = Context.RenderComponent<DynamicComponentWithErrorBoundary>(parameters =>
        {
            parameters.Add(r => r.Type, typeof(MudTextField<string>));
            // commenting out the following line (and the line that checks the Text propery value below allows the test to pass
            parameters.Add(r => r.Parameters,
                new Dictionary<string, object> {{nameof(MudTextField<string>.Text), text}});
        });
        var errorBoundary = cut.FindComponent<ErrorBoundary>();
        var textContent = errorBoundary.FindComponent<MudTextField<string>>();
        textContent.Instance.Text.Should().Be(text);
    }

Results in this output:

System.InvalidOperationException
Attributes should only be encountered within RenderElement
   at Bunit.Htmlizer.RenderCore(HtmlRenderingContext context, ReadOnlySpan`1 frames, Int32 position) in /_/src/bunit.web/Rendering/Internal/Htmlizer.cs:line 96
   at Bunit.Htmlizer.RenderFrames(HtmlRenderingContext context, ReadOnlySpan`1 frames, Int32 position, Int32 maxElements) in /_/src/bunit.web/Rendering/Internal/Htmlizer.cs:line 73
   at Bunit.Htmlizer.RenderChildComponent(HtmlRenderingContext context, ReadOnlySpan`1 frames, Int32 position) in /_/src/bunit.web/Rendering/Internal/Htmlizer.cs:line 122
   at Bunit.Htmlizer.RenderCore(HtmlRenderingContext context, ReadOnlySpan`1 frames, Int32 position) in /_/src/bunit.web/Rendering/Internal/Htmlizer.cs:line 104
   at Bunit.Htmlizer.RenderFrames(HtmlRenderingContext context, ReadOnlySpan`1 frames, Int32 position, Int32 maxElements) in /_/src/bunit.web/Rendering/Internal/Htmlizer.cs:line 73
   at Bunit.Htmlizer.RenderChildComponent(HtmlRenderingContext context, ReadOnlySpan`1 frames, Int32 position) in /_/src/bunit.web/Rendering/Internal/Htmlizer.cs:line 122
   at Bunit.Htmlizer.RenderCore(HtmlRenderingContext context, ReadOnlySpan`1 frames, Int32 position) in /_/src/bunit.web/Rendering/Internal/Htmlizer.cs:line 104
   at Bunit.Htmlizer.RenderFrames(HtmlRenderingContext context, ReadOnlySpan`1 frames, Int32 position, Int32 maxElements) in /_/src/bunit.web/Rendering/Internal/Htmlizer.cs:line 73
   at Bunit.Htmlizer.GetHtml(Int32 componentId, RenderTreeFrameDictionary framesCollection) in /_/src/bunit.web/Rendering/Internal/Htmlizer.cs:line 61
   at Bunit.Rendering.RenderedFragment.UpdateMarkup(RenderTreeFrameDictionary framesCollection) in /_/src/bunit.web/Rendering/RenderedFragment.cs:line 164
   at Bunit.Rendering.RenderedFragment.Bunit.IRenderedFragmentBase.OnRender(RenderEvent renderEvent) in /_/src/bunit.web/Rendering/RenderedFragment.cs:line 142
   at Bunit.Rendering.TestRenderer.UpdateDisplayAsync(RenderBatch& renderBatch) in /_/src/bunit.core/Rendering/TestRenderer.cs:line 177
   at Microsoft.AspNetCore.Components.RenderTree.Renderer.ProcessRenderQueue()
--- End of stack trace from previous location ---
   at Bunit.Rendering.TestRenderer.AssertNoUnhandledExceptions() in /_/src/bunit.core/Rendering/TestRenderer.cs:line 380
   at Bunit.Rendering.TestRenderer.Render[TResult](RenderFragment renderFragment, Func`2 activator) in /_/src/bunit.core/Rendering/TestRenderer.cs:line 239
   at Bunit.Rendering.TestRenderer.RenderFragment(RenderFragment renderFragment) in /_/src/bunit.core/Rendering/TestRenderer.cs:line 48
   at Bunit.Extensions.TestContextBaseRenderExtensions.RenderInsideRenderTree(TestContextBase testContext, RenderFragment renderFragment) in /_/src/bunit.core/Extensions/TestContextBaseRenderExtensions.cs:line 43
   at Bunit.Extensions.TestContextBaseRenderExtensions.RenderInsideRenderTree[TComponent](TestContextBase testContext, RenderFragment renderFragment) in /_/src/bunit.core/Extensions/TestContextBaseRenderExtensions.cs:line 23
   at Bunit.TestContext.Render[TComponent](RenderFragment renderFragment) in /_/src/bunit.web/TestContext.cs:line 66
   at Bunit.TestContext.RenderComponent[TComponent](Action`1 parameterBuilder) in /_/src/bunit.web/TestContext.cs:line 52
   at MyProject.ComponentRendererTests.ComponentRenderer_WrapsRenderedComponentInErrorBoundary() 

Expected behavior:

The test should pass without error.

Version info:

Additional context:

egil commented 2 years ago

Thanks @jimitndiaye. We'll investigate further and get back to you.

jimitndiaye commented 2 years ago

@egil Thank you. I'd like to add that doing the exact same component described above but in razor works. The following razor syntax is functionally equivalent to to what's going on in the component above:

<ErrorBoundary>
    <ChildContent>
        <DynamicComponent @ref="_dynamicComponent" Type="ComponentType" Parameters="ComponentParameters"/>
    </ChildContent>
    <ErrorContent>
        <MudStack Class="border-2 mud-border-error" AlignItems="AlignItems.Center" Justify="Justify.Center" Style="min-height: 50px">
            <MudAlert Severity="@Severity.Error">An internal UI error occurred. Please contact support.</MudAlert>
        </MudStack>
    </ErrorContent>
</ErrorBoundary>

If i run the same test but with this razor component it works, even though they are functionally doing the exact same thing - both output the exact same markup/component tree.

linkdotnet commented 2 years ago

Hmmm I might have a hunch what is causing the problem:

 builder.AddAttribute(1, nameof(ErrorBoundary.ChildContent), (RenderFragment)(childContentBuilder =>
        {
            childContentBuilder.OpenComponent(2, Type);

If Type == MudTextField<string> wouldn't be the produced markup something like:

<ErrorBoundary ChildContent="MudTextField<string>"

which is basically invalid XML. Just on top of my head did you really want to add builder.AddAttribute in this case. You wanted to have something like this @jimitndiaye , or?

<ErrorBoundary>
  <ChildContent>
    // Here your stuff
 </ChildContent>
</ErrorBoundary>
egil commented 2 years ago

Wait, the razor version works?

That's surprising. Have you looked at the code the Blazor/Razor compiler generates from the razor version to make sure they are 100% the same?

jimitndiaye commented 2 years ago

@linkdotnet your interpretation of that line of code isn't quite accurate. The code sets the ChildContent property to a RenderFragment, not Type. Like I said, the code actually works at runtime and renders the component indicated (in the test it is a MudTextField) The equivalent razor would be

<ErrorBoundary>
  <ChildContent>
    <MudTextField T="string" />
 </ChildContent>
</ErrorBoundary>
jimitndiaye commented 2 years ago

@egil I mean in blazor (WASM/Server) the razor version works and they output the exact same code (in fact i originally did it in razor then copied/tweaked the generated render code to suit my needs)

jimitndiaye commented 2 years ago

@egil The razor version I posted above uses the built-in DynamicComponent and outputs the following render code:

        #pragma warning disable 1998
        protected override void BuildRenderTree(Microsoft.AspNetCore.Components.Rendering.RenderTreeBuilder __builder)
        {
            __builder.OpenComponent<Microsoft.AspNetCore.Components.Web.ErrorBoundary>(0);
            __builder.AddAttribute(1, "ChildContent", (Microsoft.AspNetCore.Components.RenderFragment)((__builder2) => {
// this part is where it differs from the original component in the issue. instead of using DynamicComponent I just directly render the component (using the same code that DynamicComponent does)
                __builder2.OpenComponent<Microsoft.AspNetCore.Components.DynamicComponent>(2);
                __builder2.AddAttribute(3, "Type", global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<System.Type>(

                                                         ComponentType

                ));
                __builder2.AddAttribute(4, "Parameters", global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<System.Collections.Generic.IDictionary<System.String, System.Object>>(

                                                                                    ComponentParameters

                ));
                __builder2.AddComponentReferenceCapture(5, (__value) => {

                                _dynamicComponent = (Microsoft.AspNetCore.Components.DynamicComponent)__value;

                }
                );
                __builder2.CloseComponent();
            }
            ));
            __builder.AddAttribute(6, "ErrorContent", (Microsoft.AspNetCore.Components.RenderFragment<System.Exception>)((context) => (__builder2) => {
                __builder2.OpenComponent<MudBlazor.MudStack>(7);
                __builder2.AddAttribute(8, "Class", "border-2 mud-border-error");
                __builder2.AddAttribute(9, "AlignItems", global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<MudBlazor.AlignItems?>(
#nullable restore
                                                                AlignItems.Center

                ));
                __builder2.AddAttribute(10, "Justify", global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<MudBlazor.Justify?>(

                                                                                            Justify.Center

                ));
                __builder2.AddAttribute(11, "Style", "min-height: 50px");
                __builder2.AddAttribute(12, "ChildContent", (Microsoft.AspNetCore.Components.RenderFragment)((__builder3) => {
                    __builder3.OpenComponent<MudBlazor.MudAlert>(13);
                    __builder3.AddAttribute(14, "Severity", global::Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<MudBlazor.Severity>(

                                 Severity.Error

                    ));
                    __builder3.AddAttribute(15, "ChildContent", (Microsoft.AspNetCore.Components.RenderFragment)((__builder4) => {
                        __builder4.AddContent(16, "An internal UI error occurred. Please contact support.");
                    }
                    ));
                    __builder3.CloseComponent();
                }
                ));
                __builder2.CloseComponent();
            }
            ));
            __builder.CloseComponent();
        }
        #pragma warning restore 1998
egil commented 2 years ago

@egil I mean in blazor (WASM/Server) the razor version works and they output the exact same code (in fact i originally did it in razor then copied/tweaked the generated render code to suit my needs)

Ah so the razor version does not work in bUnit, but works in Blazor?

jimitndiaye commented 2 years ago

@egil no the razor version works in both. It's the code version that works in Blazor but not in bUnit. Even though they are functionally identical. The only difference is what's happening in the ErrorBoundarys ChildContent property - The razor version uses the built-in DynamicComponent to render the component while the code version just directly renders the component without going through DynamicComponent (instead just pretty much copies the code that dynamic component uses to render the component). That's what's happening here (from the original component above):

        builder.AddAttribute(1, nameof(ErrorBoundary.ChildContent), (RenderFragment)(childContentBuilder =>
        {
            childContentBuilder.OpenComponent(2, Type);
            if (Parameters is not null)
                foreach (var entry in Parameters)
                    builder.AddAttribute(3, entry.Key, entry.Value);
            childContentBuilder.AddComponentReferenceCapture(4, component =>
            {
                Instance = component;
            });

            childContentBuilder.CloseComponent();
        }
            ));
egil commented 2 years ago

Like I mentioned on Gitter, much of the code in Htmlizer is lifted from Blazors prerenderer logic, but we might need to upgrade to a newer version.

jimitndiaye commented 2 years ago

Yea I just found it interesting that functionally identical code behaves differently in bUnit. If you run either version (code or razor) in Blazor you'd see the the output is identical. Maybe upgrading the Htmlizer is the answer.

egil commented 2 years ago

Yea I just found it interesting that functionally identical code behaves differently in bUnit. If you run either version (code or razor) in Blazor you'd see the the output is identical.

Me too. Just sharing that detail from our Gitter conversation with Steven.

But we also need to make sure the two are actually functionally the same, or rather, they do not seem to be, since they behave differently, which by definition means they are not. But it's interesting none the less.

egil commented 2 years ago

Btw. Here is the original code for DynamicComponent. https://source.dot.net/#Microsoft.AspNetCore.Components/DynamicComponent.cs

jimitndiaye commented 2 years ago

@egil yea I borrow quite a lot of code from that in the above component.

jimitndiaye commented 2 years ago

The code inside this RenderFragment delegate is pretty much a lift from the Render function in DynamicComponent:

        builder.AddAttribute(1, nameof(ErrorBoundary.ChildContent), (RenderFragment)(childContentBuilder =>
        {
            childContentBuilder.OpenComponent(2, Type);
            if (Parameters is not null)
                foreach (var entry in Parameters)
                    builder.AddAttribute(3, entry.Key, entry.Value);
            childContentBuilder.AddComponentReferenceCapture(4, component =>
            {
                Instance = component;
            });

            childContentBuilder.CloseComponent();
        }
            ));
jimitndiaye commented 2 years ago

And that part is the only difference between the code version and the razor version - the code version just in-line's the Render function from DynamicComponent while the razor version just calls DynamicComponent to do it instead.

jimitndiaye commented 2 years ago

You get the error in that section where it's checking if Parameters is null. If not null you get the error. If null or empty you don't get the error.

linkdotnet commented 2 years ago

For future reference the current version of the HtmlRenderer in the aspnetcore repo: https://github.com/dotnet/aspnetcore/blob/main/src/Mvc/Mvc.ViewFeatures/src/RazorComponents/HtmlRenderer.cs

there are some smaller changes since the original version. On first sight nothing exciting. Will have a look tomorrow. We also made changes on our own, even though I don’t think they changed anything in the behaviour.

egil commented 2 years ago

@linkdotnet we also need to make sure we understand what the difference IS between the razor version @jimitndiaye posted and the handcoded C# version, because the first one does work with bUnit.

It could explain why, and also provide a hit as to how to fix this.

Once we know more, we might also reach out to the Blazor team to verify this is actually legal/expected behavior that @jimitndiaye is seeing.

linkdotnet commented 2 years ago

I found the issue. Very easy to miss (took me almost 2 hours including diffs between our HtmlRenderer and the one in the aspnetcore repo):

void Render(RenderTreeBuilder builder)
    {
        builder.OpenComponent<ErrorBoundary>(0);
        builder.AddAttribute(1, nameof(ErrorBoundary.ChildContent), (RenderFragment)(childContentBuilder =>
        {
            childContentBuilder.OpenComponent(2, Type);
            if (Parameters is not null)
                foreach (var entry in Parameters)

                    // This should be childContentBuilder and not builder
                    // So the super easy fix, which lets the test pass
                    // childContentBuilder.AddAttribute(3, entry.Key, entry.Value);
                    builder.AddAttribute(3, entry.Key, entry.Value);

The reason is simple: You use the wrong object / builder ;) Using builder instead of childContentBuilder inside your lambda will for sure result in invalid HTML ;)

linkdotnet commented 2 years ago

I would close the issue for now. If you have further questions let us know.

jimitndiaye commented 2 years ago

@linkdotnet good catch! Can't believe I missed that.