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.49k stars 10.04k forks source link

Blazor sets EditForm Model to null in unexpected situations #51420

Open halter73 opened 1 year ago

halter73 commented 1 year ago

Given the following component:

@page "/null-forms"

<h3>Null Forms</h3>

<p>@message</p>

<EditForm Model="Input" FormName="main-form" OnSubmit="OnMainSubmit">
    @if (!HideOptional)
    {
        <div>
            <label>Optional input: </label>
            <InputText @bind-Value="Input.Optional" />
        </div>
    }
    <button type="submit">Submit primary</button>
</EditForm>

<form @formname="secondary-form" @onsubmit="OnSecondarySubmit" method="post">
    <AntiforgeryToken />
    <button type="submit">Submit secondary</button>
</form>

@code {
    private string? message = null;

    [SupplyParameterFromQuery]
    private bool HideOptional { get; set; }

    // Specifying the FormName works around the bug during secondary-form submission,
    // but not the bug when there's no optional input loaded.
    //[SupplyParameterFromForm(FormName = "main-form")]
    [SupplyParameterFromForm]
    private InputModel Input { get; set; } = new();

    private void OnMainSubmit()
    {
        message = $"OnMainSubmit: {Input.Optional}";
    }

    private void OnSecondarySubmit()
    {
        message = "OnSecondarySubmit";
    }

    private sealed class InputModel
    {
        public string Optional { get; set; } = "";
    }
}

You will see an error like the following:

InvalidOperationException: EditForm requires either a Model parameter, or an EditContext parameter, please provide one of these.
Microsoft.AspNetCore.Components.Forms.EditForm.OnParametersSet()

This happens if you either click "Submit secondary" or click "Submit primary" with ?hideoptional=true. You can work around the "Submit secondary" issue by specifying [SupplyParameterFromForm(FormName = "main-form")] for the InputModel, but I'm not sure about a workaround for "Submit primary" with ?hideoptional=true other than putting Intput ??= new() in OnInitialized().

This came up when working on the Identity Razor components for the .NET 8 project templates. DeletePersonalData.razor and Email.razor

ghost commented 1 year 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.

marinasundstrom commented 1 year ago

This was annoying. I have had quite a lot of problems with the changing behavior of forms throughout the previews and RCs. This was the last one that I tackled.

oliverw commented 11 months ago

This is extremely annoying and still happening with .NET 8 RTM.

I was able to reproduce the problem reliably using a checkbox binding:

public class BillingModeForm
{
    public bool Annual { get; set; }
    public string NoOp { get; set; }
}
<EditForm FormName="billing-mode" Enhance Model="@BillingMode">
    <div class="form-check form-switch">
        <InputCheckbox class="form-check-input" role="switch" bind-Value="BillingMode.Annual"></InputCheckbox>
    </div>

    <InputText @bind-Value="BillingMode.NoOp" class="d-none"></InputText>
    <button class="btn btn-primary" type="submit">Submit</button>
</EditForm>

Without the NoOp property, the form would always be null on Post after unchecking the checkbox (switch).

oliverw commented 11 months ago

This issue should really be part of the 8.0.x milestone. It is extremely annoying to have dummy field in many of your forms just as a work around for this.

ghost commented 11 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.

mkArtakMSFT commented 10 months ago

Discussed this offline. We may end up creating an analyzer to warn customers from falling into this pitfall.

BenEskilstark commented 10 months ago

@mkArtakMSFT can you explain what the pitfall is?

I am hitting this issue -- I'm editing the template-generated Register.razor page to have an additional EditForm on it with other user info (it's a separate form because I wanted to be able to reuse it as a razor component on a user edit page as well) -- but whenever I try to "submit" data to that form (it maintains a list of claims for that user), I get the error from the OP saying I need a Model parameter even though I have one.

I'm super new to .NET in general and so I get the impression there's a lot of knowledge going on behind the scenes in this thread. Could you explain what is happening and why this is a pitfall and not just a bug?

kitgrose commented 9 months ago

So far, this has been the most frustrating issue I've run into with Blazor SSR, and the tooling provides effectively no guidance as to what is going wrong. This is especially problematic as someone attempting to adopt the framework for the first time, since I'm naturally unsure if a problem like this is an issue in my understanding of how things are supposed to work, or an issue with my code.

For me, the situation has been as follows:

  1. Create a simple form to create an entity with a simple data model ParentModel, bound to the form through the [SupplyParameterFromForm] attribute. Everything works great.
  2. Extend the data model to have a relationship to some other model (in my case, it's an ICollection<ChildModel>, which inherently has a reciprocal virtual ParentModel? member), but don't change the form at all.
  3. Notice the model is now bound to null after form submit.

The issue is resolved by adding a superfluous <input type="hidden" name="ParentModel.Id" value="0" /> element to the form (which, FWIW, feels like I'm fighting the framework, since it's the only raw <input> element in my form). My (naive) explanation for the error in retrospect is that the form binding wants to be able to populate the implicit ParentModelId member of any entities of that child model to be able to create a valid reference to the upstream parent model, but I don't understand why that should be required given I have no form fields that bind to that property of my model.

I expected that the logic for mapping would call the form's model's empty constructor, then map in the fields it observes in the form submission, so I didn't expect any behaviour change when adding new fields to my model.

Unexpected behaviour aside, this has proven incredibly frustrating to debug. I can find no easy way to see why SupplyParameterFromForm returned null, which gives me no clues as to how to solve it. It took a surprisingly long time to even figure out how to place a breakpoint to observe that problem. Despite many years working with .NET, Blazor is sufficiently abstract and new to me that it's incredibly difficult to find documentation (or code) showing how that mapping is performed, and how I would go about fixing the issue.

In general, I expected some kind of exception to be raised, or a debug message, or something inside Visual Studio to give me a hint as to why that binding failed and what I could do about it. I also expected some kind of callback I could intercept to modify (or understand) the mapping between the HttpContext and the form if I so chose (although there's every chance that such a thing exists and I've just been struggling to find it).

The issue has been significantly exacerbated by there being no samples I could find demonstrating how to handle HasMany/collection relationships in Blazor forms.

CodeFontana commented 8 months ago

I was just tripped up by this scenario, albeit silly, I would toss in for this issue:

[SupplyParameterFromForm(Name = "createCategory")]         @* Model is null *@
[SupplyParameterFromForm(FormName = "createCategory")]     @* Correct! *@

You got me Blazor Team! I lost ~20 minutes on this. I could see this being an easy pitfall for anyone!

W4lm4s commented 8 months ago

Hi,

To add my 2 cents, this issue is extremely frustrating, in the scenario of using checkbox, the default behavior is : to do not send value if the checkbox is not checked (the value being 'False'). So, it leads to and incomplete form model and then ultimately, to the model being null (because one of the field is missing).

Currently I'm having a bunch of if statements below every checkbox :

 @if(!@Model.RememberMe)
 {
     <input type="hidden" id="RememberMe" name="Model.RememberMe" value="False" />
 }

On every field I'm adding these hidden field, as pointed by @kitgrose (btw : Thank you for the workaround, it is better than mapping fields manually)

The code is looking bloated. ;(

I experience the issue with the very last version of VS2022 Community & net 8 sdk.

Hope this issue can be addressed as soon as possible.