-
Notifications
You must be signed in to change notification settings - Fork 10.3k
[Blazor] Improve auto render mode selection strategy #49858
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
this.updateAllRootComponentsAfterRendererAttached(WebRendererId.Server); | ||
this.updateAllRootComponentsAfterRendererAttached(WebRendererId.WebAssembly); |
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.
Each of these calls will wait for rendererAttached
and then call updateAllRootComponents
.
So technically, if there are both webassembly and server interactive root components, all will flicker twice? I am just wondering, if it will make sense to wait a little bit longer and call the updateRootComponents
only once?
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.
No flickering happens here. What these calls do is wait for the specific renderer to attach (which indicates that the platform is fully initialized), then call updateAllRootComponents()
. The updateAllRootComponents()
function looks at each root component to determine if:
- It's never been seen before, in which case it gets enabled for interactivity
- Or, it's been seen before, but it's parameters have changed, in which case send the updated parameters to .NET
- Or, it's been seen before, but now it's been removed from the page in an SSR update, so we dispose it in .NET
- Or, none of the above cases happen, in which case it's a no-op for the given component
It can kind of be thought of as calling StateHasChanged()
, except for that it doesn't always trigger a re-render. There isn't really a downside to calling it in cases where we don't end up needing to.
return 'server'; | ||
} | ||
|
||
return null; |
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.
How will the null
result be interpreted by the callers 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.
The null
indicates that a render mode could not be determined yet. This prevents the component from becoming interactive. At some point in the future (if WebAssembly starts up or fails to load quickly enough), the component will be activated via a call to updateAllRootComponents()
.
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.
Thanks @MackinnonBuck.
I sign off but please wait for at least somebody else to also review this and approve.
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.
I'm not very familiar with this area of code, but everything looks very logical and in line with the previous discussions regarding this issue. Also, great work on the e2e tests!
Thanks for the reviews, @surayya-MS, @pavelsavara and @mkArtakMSFT. I've just made some further improvements. If you feel inclined to re-review, I would appreciate it. Here's what I changed:
|
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.
I think the code looks cleaner now. As for the changed logic for using WebAssembly when the resources are loaded and for creating a circuit for Server later seems logical to me.
Improve auto render mode selection strategy
This PR introduces an improved auto render mode selection strategy that estimates whether WebAssembly resources are already cached.
Description
The current auto mode implementation uses a purely timeout-based mechanism for determining whether Blazor Server or Blazor WebAssembly should be used for an auto component. This PR improves the auto mode implementation to factor in whether WebAssembly resources are already cached by checking the
resources.hash
value inblazor.boot.json
. A larger, 3 second timeout still exists in case that estimation turns out to be incorrect and Blazor WebAssembly takes too long to load.Here's a summary of the changes in this PR:
Boot.Web.ts
that uses a hash inlocalStorage
to determine if WebAssembly resources are likely cachedRootComponentManager
and simplified the rules:Boot.Web.ts
to be determining the conditions under which to start the WebAssembly or Server runtimesRootComponentManager
so it can determine by itself whether an update is pending for SSR'd interactive root componentsBoot.Web.ts
callingrootComponentManager.handleUpdatedComponentDescriptors()
everywhereFixes #49580