-
Notifications
You must be signed in to change notification settings - Fork 10.3k
[Blazor] Support for determining the render mode dynamically #49477
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
d0301de
to
812ae61
Compare
52db278
to
5de9250
Compare
src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.Streaming.cs
Outdated
Show resolved
Hide resolved
|
||
function boot(options?: Partial<WebStartOptions>) : Promise<void> { | ||
if (started) { | ||
throw new Error('Blazor has already started.'); | ||
} | ||
started = true; | ||
webStartOptions = options; | ||
autoModeWebAssemblyTimeoutMilliseconds = options?.ssr?.autoModeWebAssemblyTimeoutMilliseconds ?? 100; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This 100ms default could be problematic. It looks like it ends up being a race condition. How do we know that 100ms is fast enough to get to "loaded" even if all the files are already cached? Might it depend on the speed of the user's device, and if it's too slow, they never get WebAssembly when the page first starts, even with the files downloaded?
It would be much better if this was deterministic, i.e., if the files are already cached then we resolve as wasm, and if not we resolve as server.
My guess is the reason you did it this way is that when we moved Platform.start
into dotnet.js
, we lost the ability to directly interact with the startup process and hence no longer know when it's got all the files that it wants. And so, the 100ms timeout is a rough approximation to "if the files are already cached we'll probably know about it by now". Is that right, or am I misunderstanding and we could actually know deterministically whether the files are all cached?
If my guesses here are right, I would suggest we should:
- Remove the public API that offers the timeout, so it's just hardcoded for preview7 (a short timeout is never really useful; the files are either cached already, or won't possibly get loaded within 100ms)
- File an issue for RC1 tracking the need to resolve this deterministically, without the timeout
Do you think I'm on the right lines here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My guess is the reason you did it this way is that when we moved...
Yes, that's exactly right. This was meant as a workaround because information about which files are already cached is hidden from Blazor. I'm totally fine with filing an issue to make any necessary dotnet.js
API changes to make this possible.
// earlier. | ||
handleUpdatedComponentDescriptors(); | ||
}, autoModeWebAssemblyTimeoutMilliseconds); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, I'm assuming it's because we have no choice but to go through this timeout. If I'm wrong and we could simply not do this and just resolve deterministically based on whether the files are already cached, I think that would be a much stronger and clearer starting point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent!
…de dynamically (#49578) # Support for determining the render mode dynamically Adds basic support for using the "Auto" render mode in Blazor apps. This allows the render mode for a component to resolve to either "Server" or "WebAssembly" depending on whether the WebAssembly runtime is downloaded and available on the client. Backport of #49477 ## Description In full-stack Blazor apps, components can be made interactive either via "WebAssembly" or "Server" render modes. The WebAssembly render mode has the advantage that it consumes fewer server resources, but the disadvantage that it delays "time to interactivity" because .NET WebAssembly runtime resources must be downloaded to the client first. Therefore, the "auto" render mode was introduced so that it: * Uses the WebAssembly render mode if the client already has the WebAssembly bits cached * Uses the Server render mode in other cases, while downloading WebAssembly resources in the background We also intend to add support for customizing the policy that determines which render mode "auto" mode should resolve to, but that's not included as part of this PR. Before this change, attempting to use the "auto" render mode would result in an error in the browser console and an unusable app. This PR allows customers to start experimenting with auto mode for their apps. Fixes #46397 ## Customer Impact Medium. The Auto render mode is already one of the three render modes customers can pick, but currently it just breaks the app. As we get closer to GA, we expect lots of customers to start trying out auto mode, so we'd like this feature to be made available as soon as possible so we can collect feedback and make the necessary corrections. ## Regression? - [ ] Yes - [X] No ## Risk - [ ] High - [ ] Medium - [X] Low This feature shouldn't impact functionality for existing Blazor Server/WebAssembly apps. It won't introduce a regression in the existing auto mode implementation, because the current implementation throws an exception. ## Verification - [X] Manual (required) - [ ] Automated
Support for determining the render mode dynamically
Opening as a draft for early feedback. Note that this PR currently targets the branch for #49238 because it builds on those changes. When that PR gets merged, I'll rebase this PR onto whatever branch the other one merges into.
Most of the changes here surround unifying the marker format between Server, WebAssembly, and Auto render modes. I do also have a rough draft of the logic for determining what render mode to use, but that will likely need some revision.
Fixes #46397