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

Streaming rendering is blocked by CSS Hot Reload (possibly by browserlink) #47608

Closed
SteveSandersonMS opened this issue Apr 7, 2023 · 7 comments
Assignees
Labels
area-blazor Includes: Blazor, Razor Components External This is an issue in a component not contained in this repository. It is open for tracking purposes. feature-full-stack-web-ui Full stack web UI with Blazor
Milestone

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Apr 7, 2023

The new streaming SSR feature we're adding for .NET 8 relies on the HTTP response being sent in a streaming manner. That is, as each chunk is written by middleware, it should be sent in the HTTP response to the browser without waiting for the entire response to complete.

Unfortunately, if CSS Hot Reload is enabled via this dialog:

image

... then streaming rendering no longer works. It appears that the response is being buffered until complete and then is sent all at once to the browser. This prevents the feature from being observed and tested in development.

Maybe it's browserlink

If you have CSS Hot Reload enabled, this is hooked up to VS via browserlink. You can see this in the generated HTML, as it inserts:

<!-- Visual Studio Browser Link -->
<script type="text/javascript" src="/_vs/browserLink" async="async" id="__browserLink_initializationData" data-requestId="7d7d24491f0b43b0ae0d2f2e1d39369c" data-requestMappingFromServer="false" data-connectUrl="http://localhost:59817/95a89da6d84a410493c62463299a0058/browserLink"></script>
<!-- End Browser Link -->

That markup is injected if and only if CSS Hot Reload is enabled. Perhaps the issue is with how this is achieved. If this is done by buffering the response so that something can search for </body> and insert it there, that would explain the whole problem.

It's not a problem with regular (non-CSS) hot reload

Note that if you have regular (non-CSS) hot reload, then something auto-injects <script src="/_framework/aspnetcore-browser-refresh.js"></script>. This does not block response streaming. I have verified that streaming rendering works fine with this being injected, as long as CSS Hot Reload is disabled.

Hopefully whatever technique is used here to inject aspnetcore-browser-refresh.js could also be used for the /_vs/browserLink script to fix the issue.

@SteveSandersonMS SteveSandersonMS added the feature-full-stack-web-ui Full stack web UI with Blazor label Apr 7, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI label Apr 7, 2023
@SteveSandersonMS
Copy link
Member Author

@BillHiebert Would you be the best person to look into this, and establish if the issue is browserlink? Or @jodavis, could someone on your team check whether browserlink does have the default behavior of blocking streaming responses?

@danroth27 @mkArtak CC for your reference

@SteveSandersonMS SteveSandersonMS added area-blazor Includes: Blazor, Razor Components and removed area-commandlinetools Includes: Command line tools, dotnet-dev-certs, dotnet-user-jwts, and OpenAPI labels Apr 7, 2023
@javiercn
Copy link
Member

We need to define a better way for tooling to integrate with the runtime. Right now, there is a lot of magic going on and it's hard for us (and users) to identify what is going on when there is a problem.

@SteveSandersonMS SteveSandersonMS added this to the .NET 8 Planning milestone Apr 10, 2023
@ghost
Copy link

ghost commented Apr 10, 2023

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@danroth27 danroth27 added the External This is an issue in a component not contained in this repository. It is open for tracking purposes. label Jun 1, 2023
@danroth27
Copy link
Member

This is being investigated by the browser link folks.

@danroth27
Copy link
Member

Update from @tlmii on June 22:

Quick update for everyone that was in this meeting. I've got a Proof of Concept/Draft PR up that appears to fix the issues with streaming caused by Browser Link.

It turns out that we were actually blocking on the VS side until the entire request had been sent to VS. We did that so we could use an html parser to find the real body tag. I've changed it so that the call back to VS is just to get the script, and the injection now matches what we currently do with the browser refresh script. I note that this makes it susceptible to the same issues as the browser refresh script (it's looking for exactly , no variants of that, and it could find it in the wrong place (a comment, for instance)). But at least whatever changes we do to fix one would now be applicable to both.

This isn't a small change (code wise it is relatively small, but testing coverage it is not), so we're not looking to push this in for 17.7 Preview 3. I'll get a more complete fix in as soon as VS main is 17.8 Preview 1 to give it a long runway."

@mkArtakMSFT
Copy link
Member

Moving to RC2 to validate the fix and close then.

@mkArtakMSFT mkArtakMSFT modified the milestones: 8.0-rc1, 8.0-rc2 Jul 31, 2023
@MackinnonBuck
Copy link
Member

It looks like this been fixed. I tested with 17.8.0 Preview 2.0 [34024.347.main] and the browser link script was getting injected without blocking the response.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components External This is an issue in a component not contained in this repository. It is open for tracking purposes. feature-full-stack-web-ui Full stack web UI with Blazor
Projects
None yet
Development

No branches or pull requests

5 participants