Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render(RenderFragment) doesn't re-render and has other limitations #1289

Closed
linkdotnet opened this issue Nov 22, 2023 · 9 comments
Closed

Render(RenderFragment) doesn't re-render and has other limitations #1289

linkdotnet opened this issue Nov 22, 2023 · 9 comments
Labels
bug Something isn't working investigate This issue require further investigation before closing.

Comments

@linkdotnet
Copy link
Collaborator

The current Render(@</>) function that accepts a RenderFragment has some rather strange quirks and limitations.

Here an example, that for sure should work:

[Fact]
public void ThatIsMyCoolTest()
{
  var output = string.Empty;
  var cut = Render(@<button @onclick='@(() => output = "Success")'>@output</button>);
	
  cut.Find("button").Click();
	
  cut.Find("button").TextContent.ShouldBe("Success");
}

In this case, no onclick event handler was found. Another example is in combination with @bind:

var model = new Model();
var ctx = new EditContext(model);
var cut = Render(
  @<div>
    <EditForm EditContext="ctx">
        <InputText @bind-Value="model.Property"></MyInput>
    </EditForm>
    <p>@model.Property</p>
  </div>
  );
cut.Find("input").Input("bUnit");
var pElement = cut.Find("p");
// This will fail, because the <p> element is not re-rendered
pElement.MarkupMatches("<p>bUnit</p>");
@linkdotnet linkdotnet added bug Something isn't working investigate This issue require further investigation before closing. labels Nov 22, 2023
@egil
Copy link
Member

egil commented Nov 22, 2023

I am not really surprised by this. A test component is not a real component, there is no render handler attached to it.

Did a few experiments:

@inherits TestContext
@code {
    public RazorStuff(ITestOutputHelper output)
    {
        // uses the Meziantou.Extensions.Logging.Xunit package
        Services.AddLogging(options =>
        {
            options.AddProvider(new XUnitLoggerProvider(output, new XUnitLoggerOptions
            {
                UseUtcTimestamp = true,
                IncludeScopes = false,
                IncludeCategory = true,
                IncludeLogLevel = true,
                TimestampFormat = "s"
            }));
            options.SetMinimumLevel(LogLevel.Trace);
        });
    }

    [Fact]
    public void ThatIsMyCoolTest()
    {
        var output = string.Empty;
        var cut = (IRenderedComponent<IComponent>)Render(@<button @onclick=@(() => output = "Success")>@output</button>);

        cut.Find("button").Click();
        cut.Render();

        Assert.Equal("Success", output); // passes
        Assert.Equal("Success", cut.Find("button").TextContent); // fails
    }
}
  • The event is triggered and the output variable is updated.
  • Casting to an IRenderedComponent and calling Render() again doesn't cause any additional renders, it seems.
  • Adding a Assert.Equal(2, cut.RenderCount); right after the call to Click shows that the render doesn't actually completes. It is stuck at one render.

The log output shows that the event is processed:

  Standard Output: 
2023-11-22T22:00:10 dbug [Microsoft.AspNetCore.Components.RenderTree.Renderer] Initializing root component 0 (Bunit.Rendering.RootComponent)
2023-11-22T22:00:10 dbug [Microsoft.AspNetCore.Components.RenderTree.Renderer] Rendering component 0 of type Bunit.Rendering.RootComponent
2023-11-22T22:00:10 dbug [Microsoft.AspNetCore.Components.RenderTree.Renderer] Initializing component 1 (Bunit.Rendering.FragmentContainer) as child of 0 (Bunit.Rendering.RootComponent)
2023-11-22T22:00:10 dbug [Microsoft.AspNetCore.Components.RenderTree.Renderer] Rendering component 1 of type Bunit.Rendering.FragmentContainer
2023-11-22T22:00:10 dbug [Bunit.Rendering.TestRenderer] Component 0 has been rendered.
2023-11-22T22:00:10 dbug [Bunit.Rendering.TestRenderer] The initial render of component 0 is completed.
2023-11-22T22:00:10 dbug [Bunit.Rendering.TestRenderer] Dispatching MouseEventArgs = {"Detail":1,"ScreenX":0,"ScreenY":0,"ClientX":0,"ClientY":0,"OffsetX":0,"OffsetY":0,"PageX":0,"PageY":0,"MovementX":0,"MovementY":0,"Button":0,"Buttons":0,"CtrlKey":false,"ShiftKey":false,"AltKey":false,"MetaKey":false,"Type":null} to "onclick" handler (id = 1) on component 0.
2023-11-22T22:00:10 dbug [Microsoft.AspNetCore.Components.RenderTree.Renderer] Handling event 1 of type 'MouseEventArgs'

This is the decompiled razor code:

[Fact]
public void ThatIsMyCoolTest_decompiled()
{
    var output = string.Empty;
    var cut = (IRenderedComponent<IComponent>)Render((__builder2) =>
    {
        __builder2.OpenElement(0, "button");
        __builder2.AddAttribute(1, "onclick", EventCallback.Factory.Create<MouseEventArgs>(this, () => output = "Success"));
        __builder2.AddContent(2, output);
        __builder2.CloseElement();
    });

    cut.Find("button").Click();
    cut.Render();

    Assert.Equal("Success", output);
    Assert.Equal("Success", cut.Find("button").TextContent);
}

@linkdotnet
Copy link
Collaborator Author

linkdotnet commented Nov 23, 2023

Here are some more observations:

  1. Render<IComponent>(@</>); fails with an exception
    That seems odd - given the fact that you can do (IRenderedComponent<IComponent>)Render(@</>); without any problem
  2. Re-Rendering does nothing.
var cut = (IRenderedComponent<IComponent>)Render(@</>);
cut.Render();

The problem is that the ParameterView is now empty - therefore we never trigger the if condition in FragmentContainer:

public Task SetParametersAsync(ParameterView parameters)
{
	if (parameters.TryGetValue<RenderFragment>("ChildContent", out var childContent))
	{
		renderHandle.Render(childContent);
	}

	return Task.CompletedTask;
}
  1. RenderFragment doesn't change.
    I guess one of the root problems of not re-rendering is, that the renderer deems the FragmentContainer didn't change. And that makes sense. The RenderFragment is a delegate - that stays stable / immutable over its lifetime. That said SetParameterAsync will never get invoked again because of this behavior!

@egil
Copy link
Member

egil commented Nov 23, 2023

Yeah, but then again, I am not sure if this is actually a bug and even if it is something we can do something about.

The examples we are discussing here is not really scenarios I find problematic that we don't support. Do we have test scenarios that we should support but don't?

@linkdotnet
Copy link
Collaborator Author

Well given the link from the user to SO - he wanted to test the two-way-binding of his custom component.
And I do see that as a valid scenario/approach

@egil
Copy link
Member

egil commented Nov 23, 2023

Well given the link from the user to SO - he wanted to test the two-way-binding of his custom component. And I do see that as a valid scenario/approach.

hmm, but didn't that work?

Declare a variable in the test, bind it in a component, trigger change, see variable update. The problem is/was that he also bound said variable to markup in the render fragment belonging to the test itself.

[Test]
public void TestMyInputComponent()
{
  var testModel = new Person();
  var editCtx = new EditContext(testModel);
  var cut = Render(
    @<EditForm EditContext="editCtx">
          <MyCustomInputComponent Label="Firstname" @bind-Value="testModel.Firstname"></MyCustomInputComponent>
      </EditForm>);

  var inputElement = cut.Find("input");
  inputElement.Input("John");

  // Passes
  Assert.That(testModel.Firstname, Is.EqualTo("John"));
}

This test verifies that two-way binding. That said, there certainly may be other reasons for needing to bind to markup declared outside components in a test. But I just can't think of a way we can do it. So I would not consider this a bug, but do think it is a good idea to note this limitation with a few examples in the docs.

My comment in the PR with the docs update came about because I didn't understand the case it was trying to explain.

@linkdotnet
Copy link
Collaborator Author

While the test might work with the bound model - but it still might be confusing for users.
If you just put the whole code inside a razor file in your test, it "magically" works. That is at least odd behavior.

@egil
Copy link
Member

egil commented Nov 23, 2023

Yeah, I guess it could. But that's because an external razor file is a component that is managed by the rendeeer, a test written in a .razor file is not a component, and it's definitely not managed by the renderer, and I don't see a way for it to be that.

I think it would require the test to be inside a real component and that test would then have to be triggered through a custom test runner.

I am honestly happy that variables/types declared in a test can be used to test two way binding as I show above.

But still, I agree it may be surprising to some, thus I think an explainer in the docs is warranted.

@linkdotnet
Copy link
Collaborator Author

What is more suprising is that even simple @onclick eventhandler can lead to exceptions.

Yeah, I guess it could. But that's because an external razor file is a component that is managed by the rendeeer, a test written in a .razor file is not a component, and it's definitely not managed by the renderer, and I don't see a way for it to be that.

On your page here - just arguing from the users point of view. It is odd if you move the same code out from inside the test to a separate component - it behaves different. Furthermore, this two versions might behave different as well:

@* If we are directly inheriting from ComponentBase and create the TestContext down below - the tests from the user would pass *@
@inherits TestContext 

<button @onclick"...">Click Me</button>
@code {
  [Fact]
  public void Title()
  {
    var cut = RenderComponent<ThisTestCompoinent>();
    cut.Find("button").Click();
  }
}

Just moving the code "up" does something else. And with v2 where everything is called Render this might come to an even bigger surprise.

That said - I reopened #1288

@egil
Copy link
Member

egil commented Nov 24, 2023

I see the ambiguity. My point is that people should not think of razor filled with unit tests in as components. They should just use it as a .cs file with the ability to have markup mixed together with c# inside methods.

So we want to make it very clear that the template looks like this:

@inherits TestContext
@code {
  ...
}

And that users should think of this as classes, not components. That should remove much if the ambiguity, I think.

@egil egil closed this as not planned Won't fix, can't repro, duplicate, stale Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working investigate This issue require further investigation before closing.
Projects
None yet
Development

No branches or pull requests

2 participants