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

Better caching for RazorSourceGenerator when SuppressRazorSourceGener… #23419

Merged
merged 1 commit into from
Jan 25, 2022

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Jan 13, 2022

…ator changes (#23358)

In VisualStudio SuppressRazorSourceGenerator changes when it's being invoked by the EnC and tooling.
In our current implementation, while we no-op the operation, when SuppressRazorSourceGenerator=true,
the code also evicts the valid previously cached results when SuppressRazorSourceGenerator=false by returning null values, causing us do work the next time its false.

Contributes to dotnet/aspnetcore#32867

Backport #23358 to 6.0.2xx

…ator changes (#23358)

In VisualStudio SuppressRazorSourceGenerator changes when it's being invoked by the EnC and tooling.
In our current implementation, while we no-op the operation, when SuppressRazorSourceGenerator=true,
the code also evicts the valid previously cached results when SuppressRazorSourceGenerator=false by returning null values, causing us do work the next time its false.

Contributes to dotnet/aspnetcore#32867
@dotnet-issue-labeler dotnet-issue-labeler bot added the Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch label Jan 13, 2022
@pranavkm pranavkm requested a review from TanayParikh January 13, 2022 18:40
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.

@mikes-gh
Copy link

I see the do not merge has been added. We are eagerly awaiting this fix so our large Blazor project can use hot reload in VS. At the moment its too slow to use.

@pranavkm
Copy link
Contributor Author

@mikes-gh my plan is to get this merged in the 6.0.3xx release which aligns with VS 17.2. The code involves quite a bit of how the source generator caches and there's a very real risk that we introduce subtle caching bugs that weren't caught in local testing. Putting it in the 17.2 preview builds gives us a bit more time to address bugs while gathering feedback.

@marcpopMSFT marcpopMSFT added this to the 6.0.3xx milestone Jan 20, 2022
@pranavkm pranavkm changed the base branch from release/6.0.2xx to release/6.0.3xx January 22, 2022 04:14
@pranavkm pranavkm enabled auto-merge (squash) January 22, 2022 04:15
@pranavkm pranavkm merged commit 2620b54 into release/6.0.3xx Jan 25, 2022
@TanayParikh TanayParikh deleted the prkrishn/razor-caching-backport branch January 25, 2022 00:31
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants