dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.22k stars 9.95k forks source link

.NET 8 Blazor SSR - Multiple Forms On Page - Field Binding #52360

Open sbwalker opened 10 months ago

sbwalker commented 10 months ago

Is there an existing issue for this?

Describe the bug

Create a standard component which contains a form. Include 2 instances of this component in a page component - ensuring that each component has a unique form name. Enter information into a field in the first form instance and click Submit. The field in the second form instance will be populated.

image

Expected Behavior

I would expect that the 2 forms would behave independently from one another, since they have unique form names. The field in the second form should remain blank and not be populated with the value from the first form.

Steps To Reproduce

Create app from Blazor Web template. Add StaticForm.razor to /Pages folder and modify Home.razor based on below:

Home.razor

@page "/"

<StaticForm FormName="Form1" />

<StaticForm FormName="Form2" />

StaticForm.razor

<div>Form Name: @FormName</div>

<form method="post" @onsubmit="Submit" @formname="@FormName" data-enhance>
    <AntiforgeryToken />
    <label for="name">Name:</label><br>
    <input type="text" id="name" name="name" class="form-control" @bind="@_name"><br>
    <button type="submit" class="btn btn-primary">Submit</button>
</form>

@if (!string.IsNullOrEmpty(_message))
{
    @((MarkupString)_message)

}

@code {
    [Parameter]
    public string FormName { get; set; }

    [SupplyParameterFromForm] public string Name { get => ""; set => _name = value; }

    private string _name;
    private string _message;

    private void Submit()
    {
        _message = $"<br /><p>Form Submitted...Name: {_name}</p>";
    }
}

Exceptions (if any)

No response

.NET Version

8.0.100

Anything else?

It appears that the [SupplyParameterFromForm] attribute is not considering the FormName - it is binding solely based on the name attribute of input elements.

gragra33 commented 10 months ago

Why not set the form name in a hidden field in the form itself. Then the form name would be returned with the data.

sbwalker commented 9 months ago

In reviewing the code for the [SupplyParameterFromFormAttribute] attribute:

https://github.com/dotnet/aspnetcore/blob/9efaf0e84dead340be574a27c6924efb5ffaa39b/src/Components/Web/src/SupplyParameterFromFormAttribute.cs

It appears that there is a FormName property which can be specified. I am guessing that this property enables it to differentiate between multiple forms on a page. I have never seen this property used in a Forms code example for Blazor SSR... but it almost seems like it should be mandatory to prevent the strange behavior that I documented above (which could happen on any page which contains multiple forms if they happen to have fields which are named the same).

sbwalker commented 9 months ago

The other problem based on the reproduction code example above is that the FormName property cannot be set dynamically based on the FormName parameter which is passed into the component. So I am not sure how I would be able to specify this property to make the code behave properly:

    [Parameter]
    public string FormName { get; set; }

    [SupplyParameterFromForm(FormName = FormName)] // An object reference is required for the non-static field, method, or property 'StaticForm.FormName'
    public string Name { get => ""; set => _name = value; }
javiercn commented 9 months ago

@sbwalker thanks for the repro.

There is a FormMappingScope component that can be used to uniquely differentiate a form in the page. Your form needs to consume the mapping scope and use it. Form Mapping will do the same.

We are working on the docs for it, but there are tests on the repo that show how to use it.

sbwalker commented 9 months ago

Thank you @javiercn - I found the tests in the repo... they seem to be focused on a specific "nested form" scenario but I am actually thinking about this more broadly...

A Blazor SSR application will be comprised of MANY forms - some will be global in nature (ie. a Search input in the Layout), some will be at a page level (ie. a page component with a form), and some at a component level (ie. a standard component with a form which is nested or even repeated as part of a collection).

It is not realistic to expect developers to name the input fields on their forms uniquely - so there will always be a probability that there may be multiple input fields on a page which have the same name (even if they may be part of different forms). These name clashes will result in binding issues as documented above. Which means that developers will have no choice but to implement the more elaborate methods such as specifying the FormName property or using FormMappingScope, etc... in order to ensure their overall application behaves as expected.

If I compare this to the very simple and elegant approach which can be used in Blazor Interactive applications for managing form input, this seems like a highly degraded developer experience. I am not sure if/how this problem can be solved - I am merely pointing out the challenges in Blazor SSR from a development perspective.

javiercn commented 9 months ago

If I compare this to the very simple and elegant approach which can be used in Blazor Interactive applications for managing form input, this seems like a highly degraded developer experience. I am not sure if/how this problem can be solved - I am merely pointing out the challenges in Blazor SSR from a development perspective.

In Blazor Server / Web Assembly you maintain all the state in memory. In Blazor during SSR you need to create a "serializable mapping" that persists across requests.

sbwalker commented 9 months ago

Example of an app with 2 distinct forms which exist in different components and have different form names:

MainLayout.razor:

@inherits LayoutComponentBase

<div class="page">
    <div class="sidebar">
        <NavMenu />
    </div>

    <main>
        <div class="top-row px-4">
            <form method="post" @onsubmit="Submit" @formname="Form1" data-enhance>
                <AntiforgeryToken />
                <div class="input-group">
                    <input type="text" id="name" name="name" class="form-control" @bind="@_name">
                    <button type="submit" class="btn btn-primary">Search</button>
                </div>
            </form>
        </div>

        <article class="content px-4">
            @Body
        </article>
    </main>
</div>

<div id="blazor-error-ui">
    An unhandled error has occurred.
    <a href="" class="reload">Reload</a>
    <a class="dismiss">🗙</a>
</div>

@code {
    [SupplyParameterFromForm]
    public string Name { get => ""; set => _name = value; }

    private string _name;

    private void Submit()
    {        
    }
}

Home.razor:

@page "/"

<form method="post" @onsubmit="Submit" @formname="Form2" data-enhance>
    <AntiforgeryToken />
    <label for="name">Name:</label><br>
    <input type="text" id="name" name="name" class="form-control" @bind="@_name"><br>
    <button type="submit" class="btn btn-primary">Submit</button>
</form>

@code {
    [SupplyParameterFromForm]
    public string Name { get => ""; set => _name = value; }

    private string _name;

    private void Submit()
    {
    }
}

Result - text entered in Form1 leaks into Form2 when Form1 is submitted:

image

Change [SupplyParameterFromForm] attribute to use explicit FormName property:

MainLayout.razor:

@inherits LayoutComponentBase

<div class="page">
    <div class="sidebar">
        <NavMenu />
    </div>

    <main>
        <div class="top-row px-4">
            <form method="post" @onsubmit="Submit" @formname="Form1" data-enhance>
                <AntiforgeryToken />
                <div class="input-group">
                    <input type="text" id="name" name="name" class="form-control" @bind="@_name">
                    <button type="submit" class="btn btn-primary">Search</button>
                </div>
            </form>
        </div>

        <article class="content px-4">
            @Body
        </article>
    </main>
</div>

<div id="blazor-error-ui">
    An unhandled error has occurred.
    <a href="" class="reload">Reload</a>
    <a class="dismiss">🗙</a>
</div>

@code {
    [SupplyParameterFromForm(FormName = "Form1")]
    public string Name { get => ""; set => _name = value; }

    private string _name;

    private void Submit()
    {        
    }
}

Home.razor:

@page "/"

<form method="post" @onsubmit="Submit" @formname="Form2" data-enhance>
    <AntiforgeryToken />
    <label for="name">Name:</label><br>
    <input type="text" id="name" name="name" class="form-control" @bind="@_name"><br>
    <button type="submit" class="btn btn-primary">Submit</button>
</form>

@code {
    [SupplyParameterFromForm(FormName = "Form2")]
    public string Name { get => ""; set => _name = value; }

    private string _name;

    private void Submit()
    {
    }
}

Result - text entered in Form1 is constrained to Form1 and does not leak to Form2:

image

Conclusion: There are very few scenarios where a developer should not specify the FormName property, as it ensures that field binding works consistently in a Blazor SSR application ie. without any side effects.

ghost commented 9 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

SteveSandersonMS commented 8 months ago

@javiercn @mkArtakMSFT Can you post more info about the remaining work to be done here? It was given a p1 but there's nothing indicating how this is not just by design.