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

Blazor re-render parent component reset child values #20405

Closed
JvanderStad opened this issue Apr 1, 2020 · 6 comments
Closed

Blazor re-render parent component reset child values #20405

JvanderStad opened this issue Apr 1, 2020 · 6 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Milestone

Comments

@JvanderStad
Copy link

Describe the bug

I've been having a problem with a Blazor component wrapped in a parent resetting values due to the parent component redrawing and using 'old' values.

To Reproduce

The first component updates correctly after an error. The second component also updates correctly, but the parent container will redraw itself and update the (child)component with none-updated values and so undoing the changes.

  • 77cdfc10-5f75-4418-a0f4-5fe34c9937fc

Further technical details

 Version:   3.1.201
 Commit:    b1768b4ae7

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18363
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.1.201\

Host (useful for support):
  Version: 3.1.3
  Commit:  4a9f85e9f8

.NET Core SDKs installed:
  2.0.0 [C:\Program Files\dotnet\sdk]
  2.1.4 [C:\Program Files\dotnet\sdk]
  2.1.202 [C:\Program Files\dotnet\sdk]
  2.1.504 [C:\Program Files\dotnet\sdk]
  2.1.505 [C:\Program Files\dotnet\sdk]
  2.1.602 [C:\Program Files\dotnet\sdk]
  2.1.700 [C:\Program Files\dotnet\sdk]
  2.1.701 [C:\Program Files\dotnet\sdk]
  2.1.801 [C:\Program Files\dotnet\sdk]
  2.1.802 [C:\Program Files\dotnet\sdk]
  2.2.104 [C:\Program Files\dotnet\sdk]
  2.2.401 [C:\Program Files\dotnet\sdk]
  2.2.402 [C:\Program Files\dotnet\sdk]
  3.0.101 [C:\Program Files\dotnet\sdk]
  3.1.200 [C:\Program Files\dotnet\sdk]
  3.1.201 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.1.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.All 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.1.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.0.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.0.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.9 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.1.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 2.2.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.0.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download```
@javiercn javiercn added the area-blazor Includes: Blazor, Razor Components label Apr 1, 2020
@mkArtakMSFT mkArtakMSFT added this to the Next sprint planning milestone Apr 1, 2020
@Andrzej-W
Copy link

I think your example is a little to complicated. I have not enough time to analyse it thoroughly but I believe there is no error in Blazor. Here is your markup:

<Blazor.InputControlLabel Label="LabelValue">
    <Blazor.InputControlDate @bind-Value="Date2"></Blazor.InputControlDate>
</Blazor.InputControlLabel>

You have two way data binding in InputControlDate, but this value is not updated if incorrect date is entered. On the other hand you have some events which are used to communicate between both components. Each time you enter valid or invalid value in InputControlDate InputControlLabel is rendered. Because InputControlDate is a child of InputControlLabel it has to be rendered also. Each time child component is rendered it receives parameters. Date2 still has its old value and it is passed to InputControlDate. Am I right?

@JvanderStad
Copy link
Author

JvanderStad commented Apr 2, 2020

The sample is a distillation from a much larger project with even more complexity involving generics and inheritance. I'll try to make it as simple as I can, but with the problem intact. I have added logging and I refactored and cleaned it a little bit more.
commit https://github.com/JvanderStad/blazor-rerender-bug/tree/5ed1823e3ce87d74301e8ac6b100f8ed4f0bdd48

Concepts

There are 3 working components in this sample

  • Date; a input=text, bound to DisplayValue, also holds a property Value with the DateTime? value
  • Container; is actually a CascadingValue, Date will notify Container of its presence and (error)state
  • Label; uses Container, display's any error messages from containing Date inputs

Sample

The sample in index.razor contains 2 variants.

Variant 1

Found here: https://github.com/JvanderStad/blazor-rerender-bug/blob/master/Pages/Index.razor#L70

Works as expected. You can enter a text, the onchange will trigger the setter of DisplayValue (https://github.com/JvanderStad/blazor-rerender-bug/blob/master/Components/Date/Date.razor.DisplayValue.cs#L15) and the Value property will update. The changes are also reflected in the bound DateTime? DateValue1 property (https://github.com/JvanderStad/blazor-rerender-bug/blob/master/Pages/Index.razor#L81) after a change.

If I follow the log messages, this is what happens:

image

The binding also works the other way around: If I update the property DateValue1, the changes are also reflected correctly in DisplayValue (You can test this by using the Set random date- button)

image

The binding works as expected, changes are reflected in the UI:

image

Variant 2

Found here: https://github.com/JvanderStad/blazor-rerender-bug/blob/master/Pages/Index.razor#L73

Is the same as variant 1, however this is wrapped with a Label-component

If you set the property of DateValue2 (with the button) the values changes and the binding is updated. This works correctly.

Log:
image

If I change the input=text value, the DisplayValue setter is called; The change is also executed correctly and the properties are updated.

Log:
image

As you can see in the above log, the bound property DateValue2 is updated.
Directly after the change, Container will update.

The Value-property of Date-component is set to the original value and not to the actual (updated) value.

If it would be a non-bound, I would understand the reset of the value (as described here: dotnet/AspNetCore.Docs#17178), but in this case the Value-property/parameter is bound to DateValue2 which has updated.

@JvanderStad
Copy link
Author

I have cleaned it a little bit more; commit: https://github.com/JvanderStad/blazor-rerender-bug/tree/09ad3f985a77967d8c62f5df0b9fef0e99b060c9

Less code, same problem.

image


Container.razor is a CascadingValue with RenderFragment

@code {
    [Parameter]
    public RenderFragment ChildContent { get; set; }
}

<CascadingValue Value="this">
    @ChildContent
</CascadingValue>

Label.razor has a Container with RenderFragment

@code {
    [Parameter]
    public RenderFragment ChildContent { get; set; }
}

<Container>
    @ChildContent
</Container>

Date.razor is a bound input-text

@code {
    [CascadingParameter]
    public Container Container { get; set; }
}

<input type="text" @bind="DisplayValue" />

If I combine those into this:

Index.razor

@code {
public DateTime? DateValue2 {get; set;}
}
 <Label>
        <Date @bind-Value="DateValue2" />
    </Label>

This does not seem to work

@uabarahona
Copy link
Contributor

uabarahona commented Apr 2, 2020

Triage issues are really good to learn, I believe the problem is due the pollution of events but before that I would like to mention that for parent child component they have best practice here:

https://docs.microsoft.com/en-us/aspnet/core/blazor/data-binding?view=aspnetcore-3.1#parent-to-child-binding-with-component-parameters

Sticking with your approach is also possible but we need to be careful.

Date.razor.DisplayValue.cs

public partial class Date
{
    private string _displayValue;

    [Parameter]
    public string DisplayValue
    {
        get => _displayValue;
        set
        {
            _displayValue = value;

            try
            {
                ValueChanged.InvokeAsync(DateTime.Parse(_displayValue));
            }
            catch
            {
                ValueChanged.InvokeAsync(null);
            }
        }
    }
}

Date.razor.Value.cs

public partial class Date
{
    private DateTime? _value;

    [Parameter]
    public DateTime? Value
    {
        get => _value;
        set
        {
            if (_value == value)
            {
                return;
            }

            _value = value;
            _displayValue = _value?.ToShortDateString() ?? _displayValue;
        }
    }

    [Parameter]
    public EventCallback<DateTime?> ValueChanged { get; set; }
}

The main point here is ValueChanged.InvokeAsync() will update the Value when the re-render runs, but oddly this will call set of Value twice, one with the original value and one with the new value, for that reason we will need to dot this if (_value == value).

So in your code (or copy the ones above, they work 😄)

  • The ValueChangedInvoke method is setting _value to the new one, which is preventing the if (_value == value) to do its job.
  • Date.Razor.Value:23 is only polluting events (indeed calling again the set twice but with the old value), it will need to be removed and also add _value=value before DisplayValueChangedInvoke

That is the solution, but I still believe this is an issue, why is set being called twice when the component is wrapped into <CascadingValue>?

@JvanderStad
Copy link
Author

JvanderStad commented Apr 3, 2020

@barahonajm Legend!

With your sample, dropping the EventCallback on DisplayValue and some extra state management between Value and DisplayValue I was able to make a workaround in the actual project 👍

I do think it's an issue though

@SteveSandersonMS
Copy link
Member

Glad you found the solution!

I do think it's an issue though

As far as I understand, this is the thing that's described at https://docs.microsoft.com/en-us/aspnet/core/blazor/components/?view=aspnetcore-3.1#overwritten-parameters, which explains why components shouldn't overwrite their incoming parameter values (or how it has the potential to cause unwanted effects). If I'm missing some reason why this scenario is different please let us know, ideally in a new issue with a simplified repro case.

@SteveSandersonMS SteveSandersonMS added ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. and removed investigate labels Sep 23, 2020
@ghost ghost added the Status: Resolved label Sep 23, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Oct 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Projects
None yet
Development

No branches or pull requests

6 participants