Surface warning/approach for component state

[guardrex]

Fixes #17178

Internal Review Topic (links to section)

• This could be handled without examples, but I don't advise it. It would be tricky to grok only from text ... and certainly take more text to explain without examples.
• This content may require a little further (more specific) explanation from one of the engineers. The content describes the pattern and conditions, but it really doesn't explain exactly why the behavior occurs.

[guardrex]

@oiBio I liked your "Expander" example. It's perfect for the documentation. I'm going to use it here. Would you like author byline credit for it? If so, I'll add "Tobias Bartsch" ... where would you like the link to go (e.g., your blog or your company)?

[oiBio]

@guardrex oh, thats nice :-) sure, why not. feel free to link my company (www.aveo-solutions.com)

i just looked at the example and you may need to write another Warning.

When you use a component with internal state inside a foreach loop and remove one, the internal state might be not what you want...
This is related to the diff-ing. You can work around this with the use of the @key-Attribute. See this example

Index.razor

@page "/"

<h1>Hello, world!</h1>

<b>
    Try to expand, the second element, then remove it.
</b>
@foreach (var contentItem in ContentList)
{
    <Expander>
        <div>
            @contentItem
        </div>
        <button @onclick="() => ContentList.Remove(contentItem)" @onclick:stopPropagation="true">
            Delete Me
        </button>
    </Expander>
}

<b>
    Try to expand, the second element, then remove it. 
    This is the expected behavoir (imho) - you need to use the @@key attribute
</b>
@foreach (var contentItem in ContentList2)
{
    // In real application you should use a better key | this one would throw an exception if the two identical strings exist in the list
    <Expander @key="contentItem">
        <div>
            @contentItem
        </div>
        <button @onclick="() => ContentList2.Remove(contentItem)" @onclick:stopPropagation="true">
            Delete Me
        </button>
    </Expander>
}



<button @onclick="StateHasChanged">Call StateHasChanged</button>



@code {
    List<string> ContentList = new List<string>();
    List<string> ContentList2 = new List<string>();

    protected override void OnInitialized()
    {
        ContentList.Add("First");
        ContentList.Add("Second");
        ContentList.Add("Third");


        ContentList2.Add("First #2");
        ContentList2.Add("Second #2");
        ContentList2.Add("Third #2");

        base.OnInitialized();
    }


}

Expander.razor

<div style="border: 1px solid #ebebeb; margin: 16px; border-radius: 2px" @onclick="@Toggle">
    Toggle (Expanded = @_expanded)

    @if (_expanded)
    {
        @ChildContent
    }
</div>

@code {
    [Parameter]
    public bool Expanded { get; set; }
    private bool _expanded { get; set; }

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


    protected override void OnInitialized()
    {
        _expanded = Expanded;
        base.OnInitialized();
    }

    void Toggle()
    {
        _expanded = !_expanded;
    }
}

[guardrex]

When you use a component with internal state inside a foreach loop and remove one, the internal state might be not what you want...
This is related to the diff-ing. You can work around this with the use of the @key-Attribute. See this example

Very well ... I'll surface that in here on the next commit. Thanks.

[guardrex]

Actually, I think we'll stick with the current @key coverage right now. Let's see if more devs report that. I'll hold your example for a future PR if we need another example to add to the coverage at ...

https://docs.microsoft.com/aspnet/core/blazor/components#use-key-to-control-the-preservation-of-elements-and-components

... glad u said something about that tho. I think this coverage goes better in the Components topic. We can't use the State Management topic right now because that topic is all about user state, not component state. I think I found a good spot for this, so we'll see on review how it looks.

Thanks again for your help. I'm going to drop the revised code and text in here that I started working on for reference in case we need it later (:smile: I'll lose it on this hard drive if I don't! 🙈).

---

When using a component with internal state inside a `foreach` loop of another component, the use the [`@key`](xref:mvc/views/razor#key) directive attribute to preserve the internal state of the child components.

Consider the following `Expander` component (*Expander.razor*):

<div style="border: 1px solid #ebebeb; margin: 16px; border-radius: 2px" @onclick="@Toggle">
    Toggle (Expanded = @_expanded)

    @if (_expanded)
    {
        @ChildContent
    }
</div>

@code {
    
    private bool _expanded { get; set; }

    [Parameter]
    public bool Expanded { get; set; }

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

    protected override void OnInitialized()
    {
        _expanded = Expanded;
        base.OnInitialized();
    }

    void Toggle()
    {
        _expanded = !_expanded;
    }
}

In the following component that uses the `Expander` component with internal state, expand the second element of **Expander Set #1** and then remove it. Note how the child component loses its state.

**Expander Set #2** uses the [`@key`](xref:mvc/views/razor#key) attribute directive so that `contentItem` is tracked for each `Expander` component (`@key="contentItem"`).

@page "/"

<h1>Hello, world!</h1>

<b>
    Try to expand, the second element, then remove it.
</b>

<h2>Expander Set #1</h2>

@foreach (var contentItem in _contentList)
{
    <Expander>
        <div>
            @contentItem
        </div>
        <button @onclick="() => _contentList.Remove(contentItem)" @onclick:stopPropagation="true">
            Delete Me
        </button>
    </Expander>
}

<h2>Expander Set #2</h2>

@foreach (var contentItem in _contentList2)
{
    <Expander @key="contentItem">
        <div>
            @contentItem
        </div>
        <button @onclick="() => _contentList2.Remove(contentItem)" @onclick:stopPropagation="true">
            Delete Me
        </button>
    </Expander>
}

<button @onclick="StateHasChanged">Call StateHasChanged</button>

@code {
    private List<string> _contentList = new List<string>();
    private List<string> _contentList2 = new List<string>();

    protected override void OnInitialized()
    {
        _contentList.Add("First");
        _contentList.Add("Second");
        _contentList.Add("Third");

        _contentList2.Add("First #2");
        _contentList2.Add("Second #2");
        _contentList2.Add("Third #2");

        base.OnInitialized();
    }
}

[guardrex]

Ping @SteveSandersonMS ... fairly quick to review 👀 this one. Do you want to say more about why this behavior exists? If not, this is ready to merge unless you spot something else 💩.

[SteveSandersonMS]

This looks good. Thanks @guardrex!

One suggestion: I wouldn’t phrase it as avoid state in parameters since that’s kind of impossible to avoid. Parameters are a kind of state.

Instead I’d phrase it as components shouldn’t write to their own parameter properties since such changes would be overwritten and lost whenever the parent rerenders and supplies new parameter values.