Blazor : @for loop increment the counter in-loop

[YordanYanakiev]

Describe the bug

While looping with @for the counter is been autoincrementing for each hit after the first usage inside a funtion.

To Reproduce

        @for( int c = 0; c < 1; c++ )
        {
            <li class="nav-item px-3">
                <NavLink class="nav-link" href="@(first( c ))" Match="NavLinkMatch.All" @onclick="OnClickTranslationLink">
                    <span class="oi oi-home" aria-hidden="true"></span>@(second( c ))
                </NavLink>
            </li>
        }

private string first( int f )
{
    return f.ToString();

}

private string second( int f )
{
    return f.ToString();
}

Got Exceptions? Include both the message and the stack trace
-->

Further technical details

• ASP.NET Core version : 3.1.100-preview1-014459
• Include the output of dotnet --info

PM> dotnet --info
.NET Core SDK (reflecting any global.json):
Version: 3.1.100-preview1-014459
Commit: ac3b59712d

Runtime Environment:
OS Name: Windows
OS Version: 6.3.9600
OS Platform: Windows
RID: win81-x64
Base Path: C:\Program Files\dotnet\sdk\3.1.100-preview1-014459\

Host (useful for support):
Version: 3.1.0-preview1.19506.1
Commit: bbf5542781

.NET Core SDKs installed:
2.0.2 [C:\Program Files\dotnet\sdk]
2.1.700 [C:\Program Files\dotnet\sdk]
2.1.801 [C:\Program Files\dotnet\sdk]
2.1.802 [C:\Program Files\dotnet\sdk]
3.0.100 [C:\Program Files\dotnet\sdk]
3.1.100-preview1-014459 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
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.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 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.0-preview1.19508.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.0.0 [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 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.0-preview1.19506.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 3.1.0-preview1.19506.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

• The IDE : Visual Studio 2019 Pro 16.4 preview 2

[javiercn]

@YordanYanakiev thanks for contacting us.

I'm not sure what you are trying to say. Could you provide a minimalistical repro project that reproduces the issue, what your expectation is and the current behavior you are observing?

[YordanYanakiev]

The expected results should be - the parameter of each of the functions to be always the same inside the loop, but each function receive somehow increased value of the counter (c) in the very same iteration.
Set a debug point on each return values of the both function and observe.

BlazorApp10.zip

[YordanYanakiev]

Here a screenshot

[javiercn]

@YordanYanakiev thanks for the repro.

I believe that this is an unfortunate consequence of how we create the child content. Here is a look at the compiled razor code for the loop.

I believe the issue is that the integer gets captured into the closure defined for the child content and as such it is always 10. This is most unfortunate, as its definitely counter-intuitive. I'm not sure if there is something we can do about it.

for( int c = 0; c < 10; c++ )
{
    __builder.AddContent(17, "            ");
    __builder.OpenElement(18, "li");
    __builder.AddAttribute(19, "class", "nav-item px-3");
    __builder.AddMarkupContent(20, "\r\n                ");
    __builder.OpenComponent<Microsoft.AspNetCore.Components.Routing.NavLink>(21);
    __builder.AddAttribute(22, "class", "nav-link");
    __builder.AddAttribute(23, "href", first( c )
    );
    __builder.AddAttribute(24, "Match", Microsoft.AspNetCore.Components.CompilerServices.RuntimeHelpers.TypeCheck<Microsoft.AspNetCore.Components.Routing.NavLinkMatch>(
      NavLinkMatch.All
    ));
    __builder.AddAttribute(25, "ChildContent", (Microsoft.AspNetCore.Components.RenderFragment)((__builder2) => {
        __builder2.AddMarkupContent(26, "\r\n                    <span class=\"oi oi-home\" aria-hidden=\"true\"></span>");
        __builder2.AddContent(27, second(c));
        __builder2.AddMarkupContent(28, "\r\n                ");
    }
    ));
    __builder.CloseComponent();
    __builder.AddMarkupContent(29, "\r\n            ");
    __builder.CloseElement();
    __builder.AddMarkupContent(30, "\r\n");
}

We'll discuss this within the team and see if we can do something about this or if there is an alternative way to achieve the same thing.

[YordanYanakiev]

It's actually even worse.
if You add for instance a third function

    private string third( int f )
    {
        Console.WriteLine( f );
        return f.ToString();
    }

<div class="@NavMenuCssClass" @onclick="ToggleNavMenu">
    <ul class="nav flex-column">
        @for( int c = 0; c < 1; c++ )
        {
            <li class="nav-item px-3">
                <NavLink class="nav-link" href="@(first( c ))" Match="NavLinkMatch.All" >
                    <span class="oi oi-home" aria-hidden="true"></span>@(second( c ))
                </NavLink>
            </li>
            <span>@third(c)</span>
        }
    </ul>
</div>

And call it in the very same body of the loop - the result in it will be MAYBE 0.

[javiercn]

I don't see how the result might be 0. In any case this seems to be an issue with NavLink specifically, as there is a pattern to support this (the same way Router does).

The problem here is that NavLink defines the content as a RenderFragment and that we should probably have made NavLink something like NavLink with the content being RenderFragment so that T can be passed in as context inside the loop.

We can't fix that for 3.1 I believe, but we might be able to do something in 5.0. In general the answer for this type of situation is to use templated components as described in https://docs.microsoft.com/en-us/aspnet/core/blazor/components?view=aspnetcore-3.0#templated-components

[YordanYanakiev]

As more levels of DOM objects is adding seems like the bug getting worse.
It's an ideological issue which must be noted and fixed i believe. :|

[javiercn]

A trivial way to workaround this is to put the scope inside a separate component and render the contents there. For example

<div class="top-row pl-4 navbar navbar-dark">
    <a class="navbar-brand" href="">BlazorApp10</a>
    <button class="navbar-toggler" @onclick="ToggleNavMenu">
        <span class="navbar-toggler-icon"></span>
    </button>
</div>

<div class="@NavMenuCssClass" @onclick="ToggleNavMenu">
    <ul class="nav flex-column">
        @for( int c = 0; c < 1; c++ )
        {
            <LinkItem Index="c" />
        }
    </ul>
</div>

@code {
    private bool collapseNavMenu = true;

    private string NavMenuCssClass => collapseNavMenu ? "collapse" : null;

    private void ToggleNavMenu()
    {
        collapseNavMenu = !collapseNavMenu;
    }
}

<li class="nav-item px-3">
    <NavLink class="nav-link" href="@first()" Match="NavLinkMatch.All">
        <span class="oi oi-home" aria-hidden="true"></span>@second()
    </NavLink>
</li>

@code{
    [Parameter] public int Index { get; set; }
    
    private string first()
    {
        return Index.ToString();

    }

    private string second()
    {
        return Index.ToString();
    }
}

[YordanYanakiev]

Yeah. I've actually consider this, but if the language construction offer a clear (on a first sign) approach usually peoples will go for it. This is my point.

[javiercn]

Ok, so we've discussed this within the team and apparently this is a C# behavior for which there is a trivial alternative, which is to use foreach. If you do the following you get the expected behavior:

        @foreach(var c in Enumerable.Range(0,10))
        {
            <li class="nav-item px-3">
                <NavLink class="nav-link" href="@(first( c ))" Match="NavLinkMatch.All" >
                    <span class="oi oi-home" aria-hidden="true"></span>@(second( c ))
                </NavLink>
            </li>
        }`

Also, capturing the variable inside of the loop also produces the desired effects. See

        @for( int c = 0; c < 10; c++ )
        {
            var current = c;
            <li class="nav-item px-3">
                <NavLink class="nav-link" href="@(first( current ))" Match="NavLinkMatch.All" >
                    <span class="oi oi-home" aria-hidden="true"></span>@(second( current ))
                </NavLink>
            </li>
        }

Given this, It is unlikely that we do anything here at the Razor level (as this is simply c#) and the recommendation is to use foreach or capture the loop variable in a local inside the loop scope as defined above.

I've filed an issue to track whether we add an analyzer to inform users when they run into this case. #16815

[YordanYanakiev]

I have considered it, yet it is giant issue I believe, and probably it have to be fixed or just the normal loops with counters removed as a feature for the syntax, although many components with enumerated names or so will be depending on such feature.

[Andrzej-W]

@YordanYanakiev change your code like this:

@for (int c = 0; c < 1; c++)
{
    int local_c = c; // <== Add this, and then use local variable
    <li class="nav-item px-3">
        <NavLink class="nav-link" href="@(first( c ))" Match="NavLinkMatch.All">
            <span class="oi oi-home" aria-hidden="true"></span>@(second(local_c))
        </NavLink>
    </li>
}

Notice that you have to use local variable only in places where it will be captured into the closure. Of course you can use it also in other places, but it is not necessary. Similar problems where reported here #15633 and here #16140. It is documented also at the end of this section:
https://docs.microsoft.com/en-us/aspnet/core/blazor/components?view=aspnetcore-3.0#lambda-expressions

I have to agree with @javiercn that in case of components with child content it is not obvious that in final code we will have closure.

[Andrzej-W]

It looks that I was to slow with my answer and Javier was faster.

[YordanYanakiev]

I would actually suggest the internal compiler to create a local variable inside the normal loop with counter and use it instead of the counter, when it is used in razor statement. This will solve the problem above.

[javiercn]

We wouldn't declare the variable locally, Blazor transpiles to C# in the closest way possible and doesn't have insight into the C# code that you write in the page while its compiling to it, and even if it had, we wouldn't alter the user code in that way.