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

feat: Add support for updatedTypes in metadata updates #40416

Merged
merged 3 commits into from
Jun 28, 2024

Conversation

jeromelaban
Copy link
Contributor

@jeromelaban jeromelaban commented Apr 24, 2024

Related dotnet/aspnetcore#52937, corresponding PR in blazor: dotnet/aspnetcore#55347

This PR passes the updatedType parameter in order by the hot reload agent to properly invoke MetadatatUpdateHandler types.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch untriaged Request triage from a team member labels Apr 24, 2024
Copy link
Contributor

Thanks for your PR, @jeromelaban.
To learn about the PR process and branching schedule of this repo, please take a look at the SDK PR Guide.

@jeromelaban
Copy link
Contributor Author

Note, I'm not sure how to test this properly on the CI's side, considering it requires an integration from the Blazor as well. Thanks for the reviews!

Copy link
Member

@MackinnonBuck MackinnonBuck left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, @jeromelaban!

We may also need to update this struct to have an UpdatedTypes property and update these tests to verify it.

@jeromelaban jeromelaban force-pushed the dev/jela/updatedtypes-hr branch from 082183a to ad90515 Compare May 9, 2024 14:52
@jeromelaban jeromelaban marked this pull request as ready for review May 9, 2024 15:31
@jeromelaban jeromelaban requested a review from a team as a code owner May 9, 2024 15:31
@MackinnonBuck
Copy link
Member

I've tested these changes locally; with a small fix, hot reload works and the updatedTypes argument gets correctly supplied to MetadataUpdateHandlers 🙂

I also verified that:

  • Using the latest SDK with a project targeting an older TFM still works
    • This is because the extra arguments passed to the Blazor._internal.applyHotReload function are simply ignored in that case
  • The changes in the aspnetcore PR work without the changes here
    • This is because I also made a small change over there allowing the updatedTypes argument to be null
    • This should allow us to merge both changes at the same time and not have to worry about dependency flow issues

@MackinnonBuck
Copy link
Member

Thanks again for the contribution, @jeromelaban!

@MackinnonBuck
Copy link
Member

(Just going to wait for the aspnetcore PR to pass all its checks to make extra sure that both ends are good-to-go before merging).

@MackinnonBuck MackinnonBuck merged commit 0097979 into dotnet:main Jun 28, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants