Skip to content

Cant use asp-for tag helpers with nullable model #40327

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

Open
1 task done
G1Vh opened this issue Feb 21, 2022 · 6 comments
Open
1 task done

Cant use asp-for tag helpers with nullable model #40327

G1Vh opened this issue Feb 21, 2022 · 6 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one Priority:2 Work that is important, but not critical for the release
Milestone

Comments

@G1Vh
Copy link

G1Vh commented Feb 21, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

After #37510 we can mark model as nullable like @model MyModel?, but then using asp-for tag helpers
<input asp-for="Id" type="hidden">
<label asp-for="Name" class="d-flex w-50"></label>
shows "Dereference of a possibly null reference" during view compilation.

When model not marked nullable and real passed model is null - inputs renders correctly:

<input type="hidden" data-val="true" data-val-required="The Id field is required." id="Id" name="Id" value="">
<label class="d-flex w-50" for="Name">TestName</label>

Probably problem is LabelTagHelper.ModelExpression and InputTagHelper.ModelExpression is not nullable but can really accept null (#5680 shows that Microsoft.AspNetCore.Mvc.TagHelpers not annotated)

Expected Behavior

View compiled without errors and renders correct inputs

Steps To Reproduce

  1. Mark page model as nullable
  2. Create input or label asp-for for any model field

Exceptions (if any)

Dereference of a possibly null reference

.NET Version

6.0.200

Anything else?

No response

@mkArtakMSFT mkArtakMSFT added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Feb 21, 2022
@iamzm
Copy link

iamzm commented Feb 21, 2022

@G1Vh is there compile or runtime error,

@G1Vh
Copy link
Author

G1Vh commented Feb 21, 2022

Compile

@pranavkm
Copy link
Contributor

This error makes sense - asp-for="Id" is equivalent to asp-for="Model.Id" where Model is nullable like you declared. Mvc handles traversing the tree in a null-safe way (or simply catches null-ref if it's too hard). Unfortunately because Model.Id is an ExpressionTree, you cannot use a null-coalescing operator e.g. Model?.Id since it's unsupported within expression trees. Using the null-forgiveness operator does work though, so consider using it: Model!.Id

@G1Vh
Copy link
Author

G1Vh commented Feb 21, 2022

This error makes sense - asp-for="Id" is equivalent to asp-for="Model.Id" where Model is nullable like you declared. Mvc handles traversing the tree in a null-safe way (or simply catches null-ref if it's too hard). Unfortunately because Model.Id is an ExpressionTree, you cannot use a null-coalescing operator e.g. Model?.Id since it's unsupported within expression trees. Using the null-forgiveness operator does work though, so consider using it: Model!.Id

As temporal workaround it works, yes.

But it still be wrong behavior, compiler should allow writing asp-for="Id" in nullable context

@SteveSandersonMS SteveSandersonMS added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Feb 21, 2022
@SteveSandersonMS SteveSandersonMS added this to the .NET 7 Planning milestone Feb 21, 2022
@ghost
Copy link

ghost commented Feb 21, 2022

Thanks for contacting us.

We're moving this issue to the .NET 7 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
Copy link
Member

cc @mkArtakMSFT @pranavkm for consideration for .NET 7

@captainsafia captainsafia modified the milestones: .NET 8 Planning, Backlog Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates enhancement This issue represents an ask for new feature or an enhancement to an existing one Priority:2 Work that is important, but not critical for the release
Projects
None yet
Development

No branches or pull requests

10 participants