Skip to content
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

Avoid declaring RazorPage<T>.Model as nullable by default (#39332) #39416

Merged
merged 1 commit into from Jan 10, 2022

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Jan 10, 2022

Description

The Model property for RazorPage<T>, the base class for MVC views, is declared as nullable. Having a nullable property reflects an accurate state when the model is not initialized as part of MVC's internal infrastructure. However, having the property be nullable is frustrating for user code when they specify a non-null model. We've had reports of users getting hundreds of errors in their app when they migrate to .NET 6 (which is when this API was annotated for nullability) with nullability enabled.

Fixes #37510

Customer Impact

Difficulty in migrating to .NET 6.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jan 10, 2022
@ghost ghost added this to the 6.0.x milestone Jan 10, 2022
@ghost ghost added this to In Progress in Servicing Jan 10, 2022
@ghost
Copy link

ghost commented Jan 10, 2022

Hi @pranavkm. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@pranavkm pranavkm added the Servicing-consider Shiproom approval is required for the issue label Jan 10, 2022
@ghost
Copy link

ghost commented Jan 10, 2022

Hi @pranavkm. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

Copy link
Contributor

@TanayParikh TanayParikh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving backport

@pranavkm
Copy link
Contributor Author

@dotnet/aspnet-build could you help with the publicAPI analyzer file?

@BrennanConroy
Copy link
Member

could you help with the publicAPI analyzer file?

You mean ignoring the "modified in a patch branch" issue? Once this is servicing approved sure.

@pranavkm pranavkm added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jan 10, 2022
@pranavkm
Copy link
Contributor Author

This was approved over email. Could I get some help getting this merged?

@wtgodbe wtgodbe merged commit 2bd9de6 into dotnet:release/6.0 Jan 10, 2022
Servicing automation moved this from In Progress to Done Jan 10, 2022
@ghost ghost modified the milestones: 6.0.x, 6.0.2 Jan 10, 2022
@wtgodbe
Copy link
Member

wtgodbe commented Jan 10, 2022

Only failure is the expected API file check

Elanis added a commit to Dysnomia-Studio/dysnomia-website that referenced this pull request Jul 14, 2023
Bumps [Microsoft.AspNetCore.TestHost](https://github.com/dotnet/aspnetcore) from 6.0.1 to 6.0.2.
<details>
<summary>Release notes</summary>

*Sourced from [Microsoft.AspNetCore.TestHost's releases](https://github.com/dotnet/aspnetcore/releases).*

> ## .NET 6.0.2
> [Release](https://github.com/dotnet/core/releases/tag/v6.0.2)
</details>
<details>
<summary>Commits</summary>

- [`1dcf7ac`](dotnet/aspnetcore@1dcf7ac) [internal/release/6.0] Update dependencies from dnceng/internal/dotnet-efcore...
- [`608229f`](dotnet/aspnetcore@608229f) [internal/release/6.0] Update dependencies from dnceng/internal/dotnet-runtim...
- [`eb1c0a4`](dotnet/aspnetcore@eb1c0a4) Merge in 'release/6.0' changes
- [`ef499fe`](dotnet/aspnetcore@ef499fe) Revert "[release/6.0] Switch to Windows.Amd64.Server2022.Open ([#39364](dotnet/aspnetcore#39364))" ([#39476](dotnet/aspnetcore#39476))
- [`5e8131b`](dotnet/aspnetcore@5e8131b) Merged PR 20577: Fix build errors
- [`04a113d`](dotnet/aspnetcore@04a113d) Merge in 'release/6.0' changes
- [`2bd9de6`](dotnet/aspnetcore@2bd9de6) Avoid declaring RazorPage\<T>.Model as nullable by default ([#39332](dotnet/aspnetcore#39332)) ([#39416](dotnet/aspnetcore#39416))
- [`6ffe8df`](dotnet/aspnetcore@6ffe8df) Merge in 'release/6.0' changes
- [`d9521ac`](dotnet/aspnetcore@d9521ac) Don't exclude System.IO.Pipelines from TargetingPack version check ([#39302](dotnet/aspnetcore#39302))
- [`3b49ab1`](dotnet/aspnetcore@3b49ab1) Merge in 'release/6.0' changes
- Additional commits viewable in [compare view](dotnet/aspnetcore@v6.0.1...v6.0.2)
</details>

<br />

Co-authored-by: Elanis <elanis@hotmail.com>
Reviewed-on: https://gitea.dysnomia.studio/elanis/dysnomia-website/pulls/26
Co-authored-by: elanis <elanis@noreply.example.org>
Co-committed-by: elanis <elanis@noreply.example.org>
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 Servicing-approved Shiproom has approved the issue
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants