Skip to content

Hot reload script injection improvements #27537

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

Conversation

MackinnonBuck
Copy link
Member

Hot reload script injection improvements

Summary

Improved hot reload script injection to be more reliable and provide better log messages when failure occurs.

Description

Implemented the following improvements to hot reload script injection:

  • The closing body tag will now be identified...
    • if some characters are uppercase or extra whitespace is inserted.
    • if the tag spans across multiple writes to the response stream (this is achieved by buffering the entire response before performing scirpt injection).
  • New log messages will...
    • suggest disabling response compression if script injection fails and the Content-Encoding header exists.
    • suggest manually adding the script tag if the cause for failure is unknown.

Fixes dotnet/aspnetcore#41772

@ghost
Copy link

ghost commented Aug 29, 2022

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@MackinnonBuck MackinnonBuck added the Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch label Aug 29, 2022
Copy link
Member

@javiercn javiercn left a comment

Choose a reason for hiding this comment

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

Looks great!

@MackinnonBuck MackinnonBuck enabled auto-merge (squash) August 30, 2022 23:19
@MackinnonBuck MackinnonBuck disabled auto-merge August 31, 2022 21:04
@MackinnonBuck MackinnonBuck merged commit 7d4ae9a into release/7.0.1xx Sep 1, 2022
@MackinnonBuck MackinnonBuck deleted the mbuck/7.0.1xx/hot-reload-script-injection-improvements branch September 1, 2022 17:03
MackinnonBuck added a commit that referenced this pull request Nov 21, 2022
mkArtakMSFT pushed a commit that referenced this pull request Jan 4, 2023
…ng (#29152)

## Summary

Fixes a regression in memory consumption caused by large responses when debugging with hot reload enabled.

## Description

As one of the changes introduced in .NET 7 by #27537, the hot reload script injection middleware now buffers the entire response before performing script injection. This was done to improve script injection reliability, but we recently began receiving reports from customers that were confused by the increase in server memory usage for large responses when debugging with hot reload is enabled.

This PR reverts the buffering behavior but keeps the logging improvements that help customers identify the cause of script injection failures and take steps to fix them.

For .NET 8, we will assess an alternative approach to improve script injection reliability that doesn't involve buffering each response.

## Validation

Manual validation + existing automated tests for this scenario.

## Risk

Low. This PR simply reverts the offending commit and adds back some of the improved logging behavior with no other changes to functionality.

Fixes dotnet/aspnetcore#45037
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.

2 participants