Blazor change detection fires for a property that does not change

[Dotneteer]

Bug description:

In the following example (created from the default Blazor project), the Index component passes an input parameter, Initial, to Counter, and gets a notification whenever the counter value changes:

@page "/"

<Counter Initial="@Initial" Changed="@OnChanged"/>

@code 
{
int Initial = 7;

void OnChanged(CounterChangedArgs e)
{
    Console.WriteLine($"Value: {e.Value}, Initial: {Initial}");
}

Counter sets its starting value in the OnParametersSet lifecycle method. Whenever the counter's value changes, it sends a notification:

@page "/counter"

<h1>Counter</h1>
<p>Current count: @currentCount</p>
<button class="btn btn-primary" @onclick="@IncrementCount">Click me</button>

@code {
    int currentCount = 0;

    [Parameter]
    int Initial { get; set; }

    [Parameter]
    EventCallback<CounterChangedArgs> Changed {get; set; }

    protected override void OnParametersSet()
    {
        Console.WriteLine("OnParameterSet invoked.");
        currentCount = Initial;
    }

    async Task IncrementCount()
    {
        currentCount++;
        await Changed.InvokeAsync(new CounterChangedArgs(currentCount));
    }
}

Every time Counter raises the Changed event, Blazor invokes the OnParametersSet() method, though the single Initial input parameter of Counter does not change at all. If Changed is not raised or Index does not handle this event callback, everything works fine.

To Reproduce

Steps to reproduce the behavior:

1. Compile and run the attached sample project: EventCallbackIssue.zip
2. Use Developer Tools in browser to look at the console log. After three clicks it contains these messages:

WASM: OnParameterSet invoked.
WASM: Value: 8, Initial: 7
WASM: OnParameterSet invoked.
WASM: Value: 8, Initial: 7
WASM: OnParameterSet invoked.
WASM: Value: 8, Initial: 7
WASM: OnParameterSet invoked.

Expected behavior

I would expect this output (OnParametersSet invoked only once, as Initial does not change):
WASM: OnParameterSet invoked.
WASM: Value: 8, Initial: 7
WASM: Value: 9, Initial: 7
WASM: Value: 10, Initial: 7

[Andrzej-W]

In the doc we can read

OnParametersSetAsync and OnParametersSet are called when a component has received parameters from its parent and the values are assigned to properties. These methods are executed after component initialization and each time the component is rendered

It looks that it works as designed and documented. You can add a private bool variable firstParameterSet and change your method to something like this:

private bool firstParametersSet = true;
protected override void OnParametersSet()
{
    if (firstParametersSet)
    {
        firstParametersSet = false;
        Console.WriteLine("OnParameterSet invoked.");
        currentCount = Initial;
    }
}

[Dotneteer]

@Andrzej-W, I know that your suggestion is a good work-around, and I'm also aware of the fact you have cited from the documentation :-).

In the sample I attached, it is not clear why Blazor re-renders the Counter component when its Changed event is fired and processed by its parent, but not when either the event is not fired or the parent does not process it. This is why I suspect it is a bug.

Do you know why it happens this way?

[Andrzej-W]

Sorry @Dotneteer - now I think that I understand what you are asking for.

Blazor re-renders component after each event. Here Counter component handles onclick and is automatically re-rendered after each mouse click. This is expected because internal state of the component changes and you want to see a new counter value on the screen. Parent component (Index in this case) doesn't have to be re-rendered because there is no chance that its internal state will change (and if it is possible you have to call StateHasChanged() yourself)

On the other hand, when you handle Change event and it is fired Index component have to be re-rendered. Usually you subscribe to events because you want to mutate the internal state of the component. This mutation can potentially change the value of Initial variable and because of this Counter component has to be re-rendered with new parameter values. It doesn't matter that in your case the new value is the same as old value.

Now I think that Blazor works as expected, but not as documented. This doc:

OnParametersSetAsync and OnParametersSet are called when a component has received parameters from its parent and the values are assigned to properties. These methods are executed after component initialization and each time the component is rendered.

should be changed to something like this:

OnParametersSetAsync and OnParametersSet are called when a component has received parameters from its parent and the values are assigned to properties. This happens when parent component is rendered. These methods are executed after component initialization.

I will open a new issue in https://github.com/aspnet/AspNetCore.Docs

[Dotneteer]

@Andrzej-W, thanks for your explanation, it sounds absolutely logical. According to it, I would not call this phenomenon a bug, rather sub-optimal behavior. I guess, before re-rendering the parent, it can be checked if there's real change (and not just potential) in the state. Probably this check is cheaper than re-rendering :-).

[Andrzej-W]

I guess, before re-rendering the parent, it can be checked if there's real change (and not just potential) in the state. Probably this check is cheaper than re-rendering :-).

This is not necessarily true. You can pass complex objects as parameters, for example List<T> where T is complex class with references to other classes. Component can be relatively simple - it displays only the number of elements in the list. In that case it is easier and cheaper to re-render the component than keep copy of this list somewhere and compare all (maybe thousands) elements recursively (recursively because each object can have references to other objects).

[Dotneteer]

Agree with you, this check can be very expensive in many cases, especially for complex object graphs. Nonetheless, there are very simple cases, especially when you need to compare intrinsic .NET value types.

I can accept Blazor works as it is, however, I'm happy to mention where I see some opportunity for performance optimization :-).

Thanks for your explanation and reporting the documentation issue!

[mkArtakMSFT]

Thanks for contacting us, @Dotneteer.
And glad to see community (@Andrzej-W in this case :) ) help!

If this comes up in our performance testing (#10449) we'll look into it as needed.