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

[release/9.0] Update HtmlAttributePropertyHelper to correctly follow the MetadataUpdateHandlerAttribute contract #59908

Open
wants to merge 1 commit into
base: release/9.0
Choose a base branch
from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Jan 16, 2025

Backport of #58558 to release/9.0

/cc @MackinnonBuck

Update HtmlAttributePropertyHelper to correctly follow the MetadataUpdateHandlerAttribute contract

Fixes an issue where, when running a Blazor, Razor Pages, or MVC app in .NET 9 via dotnet watch, a warning gets logged on each hot reload edit.

Description

A change was made in the .NET 9 SDK that logs a warning at runtime when a type claiming to implement hot reload functionality does not correctly follow the MetadataUpdateHandlerAttribute contract. This change revealed that the HtmlAttributePropertyHelper class had incorrectly implemented this contract since .NET 7 (since a warning was getting logged on each hot reload edit). Some customers also claim that this bug worsens hot reload performance by adding additional delays between hot reload edits, although this detail is only anecdotal.

The fix is simple: Subtly change the signature of a method in the HtmlAttributePropertyHelper class to conform to the metadata update handler contract.

Fixes https://github.com/dotnet/AspNetCore-ManualTests/issues/3202

Customer Impact

The biggest known impact is that Blazor, Razor Pages, and MVC apps running with dotnet watch will log a warning each time a hot reload edit is made.

In addition, since the HtmlAttributePropertyHelper's reflection cache was not getting cleared after a hot reload edit, certain hot reload edits may either 1) not apply or 2) cause an exception to get thrown (especially when utilizing the HtmlHelper.AnonymousObjectToHtmlAttributes() API). We don't currently have data on how common that particular issue is in practice. This would have been an issue since .NET 7.

Some customers also claim that this bug worsens hot reload performance, but this detail is only anecdotal.

Regression?

  • Yes
  • No

The newly-logged warning is a regression from .NET 8.

However, the reflection cache failing to clear is not a regression, as this has been an issue since hot reload support was initially added for this type in .NET 7. However, it's not clear what the customer impact of this problem is.

Risk

  • High
  • Medium
  • Low

The change is extremely trivial. Manual testing vendors have validated that the fix resolves the warning.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@github-actions github-actions bot requested a review from a team as a code owner January 16, 2025 21:47
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Jan 16, 2025
@dotnet-policy-service dotnet-policy-service bot added this to the 9.0.x milestone Jan 16, 2025
@MackinnonBuck MackinnonBuck added Servicing-consider Shiproom approval is required for the issue Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jan 16, 2025
@MackinnonBuck
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants