Skip to content

Conversation

@chsienki
Copy link
Member

@chsienki chsienki commented May 8, 2025

Ensure we write out the mapping for @model types in razor pages.

Commit by commit should hopefully make the fix pretty self-explanatory.

Fixes: #11329

@chsienki chsienki marked this pull request as ready for review May 12, 2025 22:07
@chsienki chsienki requested review from a team as code owners May 12, 2025 22:07
@chsienki
Copy link
Member Author

@dotnet/razor-compiler For reviews please :)

@chsienki chsienki added this to the 2025-05 May milestone May 12, 2025
@333fred
Copy link
Member

333fred commented May 12, 2025

Can you please rename the PR for clarity?

Copy link
Contributor

@ryzngard ryzngard left a comment

Choose a reason for hiding this comment

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

tooling LGTM

#nullable restore
public TestFiles_IntegrationTests_CodeGenerationIntegrationTest_IncompleteDirectives Model => ViewData.Model!;
#nullable disable
public TestFiles_IntegrationTests_CodeGenerationIntegrationTest_IncompleteDirectives Model => ViewData.Model;
Copy link
Member

Choose a reason for hiding this comment

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

This looks suspicious. Previously the Model property here was nullable-enabled, now it's nullable-disabled, isn't that a regression?

Copy link
Member Author

Choose a reason for hiding this comment

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

The Model property in this case isn't coming from the user, if you look it's the type of name of the file which is generated code, declared outside of the users nullability annotations. So, it's correct that it shouldn't be nullable annotated.

@chsienki chsienki changed the title Fix 11329 Ensure @model types have a mapping in razor pages May 13, 2025
Copy link
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

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

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Cohosting] Model directive contents don't get classified as expected

6 participants