aspnetcore: Blazor Two Way Binding Error

Watching the community stand up just now @danroth27 said to point out what needs attention in issues. This is one!

I’ve played around with the code and added some Console.WriteLine. This reveals the following outputs:

For Component1 without cascading value:

Child: Value set invoked with value = 0
Child: OnParametersSet
Child: OnAfterRender
Child: Value set invoked with value = 1
Child: Value setting. Old value = 0
Child: Value set. New value = 1
Parent: Value updated to 1
Child: ValueChanged invoked
Child: Value changed to 1
Child: Value set invoked with value = 1
Child: OnParametersSet
Child: OnAfterRender

And with cascading value it becomes

Child: Value set invoked with value = 0
Child: OnParametersSet
Child: OnAfterRender
Child: Value set invoked with value = 1
Child: Value setting. Old value = 0
Child: Value set. New value = 1
Parent: Value updated to 1
Child: ValueChanged invoked
Child: Value changed to 1
Child: Value set invoked with value = 0
Child: Value setting. Old value = 1
Child: Value set. New value = 0
Parent: Value updated to 0
Child: ValueChanged invoked
Child: OnParametersSet
Child: Value set invoked with value = 0
Child: OnParametersSet
Child: Value set invoked with value = 0
Child: OnParametersSet
Child: Value set invoked with value = 0
Child: OnParametersSet
Child: OnAfterRender
Child: OnAfterRender
Child: OnAfterRender

The diff is therefore:

Child: Value set invoked with value = 0
Child: OnParametersSet
Child: OnAfterRender
Child: Value set invoked with value = 1
Child: Value setting. Old value = 0
Child: Value set. New value = 1
Parent: Value updated to 1
Child: ValueChanged invoked
Child: Value changed to 1
- Child: Value set invoked with value = 1
+ Child: Value set invoked with value = 0
+ Child: Value setting. Old value = 1
+ Child: Value set. New value = 0
+ Parent: Value updated to 0
+ Child: ValueChanged invoked
Child: OnParametersSet
+ Child: Value set invoked with value = 0
+ Child: OnParametersSet
+ Child: Value set invoked with value = 0
+ Child: OnParametersSet
+ Child: Value set invoked with value = 0
+ Child: OnParametersSet
Child: OnAfterRender
+ Child: OnAfterRender
+ Child: OnAfterRender

This to me shows the bug quite clearly: The parent receives the correct value, yet the child component is subsequently updated to the original value again.

The way I see it, the loop back to setting values in the component is unavoidable. However, it shouldn’t reset any values. The order of things should be

1. change the value of the property
2. invoke ValueChanged with the new value
3. Parent component receives the event callback, and updates its value
4. Parent component re-renders, as state has changed
5. This implicitly causes the child component to get updated values. At this point, the child should receive the changed value, not the old value (this seems to me where the bug is)
6. The child component receives the new value. Here it is either responsibility of the framework to not change values that didn’t change (that isn’t yet the case, right? Probably not a universally correct thing to do?) or the programmers responsibility to have the safeguard if (backing_field == value) { return; } in the setter of the property.

In conclusion, the following code should never fail:

private T _value;
[Parameter] public T Value
{
   get => _value;
   set
   {
      if (value == _value) { return; }
      _value = value;
      ValueChanged.InvokeAsync(_value);
   }
}

This can’t result in an infinite loop, because the condition in the setter breaks it, under the assumption that the value that is bound in the parent component is updated to the new value before the re-render and the re-setting of the parameters happens.

OK, I’ve looked in more detail and see what’s going on. There’s an explanation below, but before getting to that, I’ll restate my claim that the core problem is the child component overwriting its own [Parameter] property value. By avoiding doing that, you can avoid this problem.

In this instance, and in others, Blazor relies on parent-to-child parameter assignment being safe (not overwriting info you want to keep) and not having side-effects such as triggering more renders (because then you could be in an infinite loop). We should probably strengthen the guidance in docs not just to say “don’t write to your own parameters” and expand it to caution against having side-effects in such property setters. Blazor’s use of C# properties to represent the communication channel from parent to child components is pretty convenient for developers and looks good in source code, but the capacity for side-effects is problematic. We already have a compile-time analyzer that warns if you don’t have the right accessibility modifiers on [Parameter] properties - maybe we should extend it to give warnings/errors if the parameter is anything other than a simple { get; set; } auto property.

So in this case the solution is pretty simple: replace Component1’s Value property with a simple { get; set; } one, and the instead of trying to write to it, have Component1 respond to button clicks by triggering ValueChanged. Then you don’t have any recursive render cycle, and no data overwriting occurs.

    private void DecrementValue()
    {
        //Value--; <-- Don't do this

        // Do this instead:
        ValueChanged.InvokeAsync(Value - 1);
    }

Behind the scenes

The problem you observe is because, when there’s a <CascadingValue>, there are two different sources of parameters for the child:

• The <CascadingValue> component
• The parent component

When a rendering cycle occurs, one of these has to supply values before the other, and in your case it happens to be the <CascadingValue>. For consistency, when CascadingValue supplies updated parameters to a subscriber, the subscribee receives not only the cascaded parameter value but also a snapshot of whatever previous set of parameters it received. This is so that you don’t have to worry about implementing any special-case logic to allow for parameters being missing. In the event that you’re modifying a parameter value during the same cycle as a cascaded value is propagating, the parameter snapshot will still contain the previous value. Normally this causes no problems because, as long as setting the parameter values is not side-effecting, the renderer will continue and also supply the updated values from the parent that’s re-rendering, and so you end up with a consistent and correct set of final values. The problem only occurs because setting the parameter values in the repro case does have side-effects which include triggering another re-render on the parent. The solution therefore is to leave the parameter communication channel alone (i.e., don’t write to the properties, nor put in set logic that has side-effects).

I hope all this makes sense. If you think I’m still missing something, please let me know. The actions I plan to take on this are:

• Improve docs to clarify the requirements around parameter passing
• Add a backlog item to consider a compile-time analyzer to give warnings if a [Parameter] property isn’t just { get; set; } or if it’s written to from inside the component

@stefanloerwald added a separate page (from the menu) with his logging output and this is merged into the repo. Please run the solution without IIS Express and you’ll see his logging results in the command window that shows the results of dotnet run

I too have experienced this issue and have spent endless hours trying to understand. If this isn’t considered a bug it will require some excellent documentation as the behavior is quite contrary to what one would expect. I hardly expect this sequence:

• if I change a parameter value in a component

• signal a value change to the parent

• then the parent first sets the parameter to the old value

Maybe the key to this is in this line ? “…under the assumption that the value that is bound in the parent component is updated to the new value before the re-render and the re-setting of the parameters happens”

I mean I don’t know for sure, but I do know it felt really bad to be doing it that way - and changing it the way I did superficially confirmed it as a reasonable concern - because it works the other way.

I would also note that I would never “double-bind” like this example (where the incoming parameter is then bound inside the component) anyway.

I always bind the backing field to the html element in my component - as in my example - and so I have never had this problem.

So - TLDR; - The reason I didn’t personally add my example here is because I am just another user and didn’t want to sidetrack this issue with what could be considered a workaround.