-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Make sure WebRootComponentManager is initialized for dynamic JS roots
#64467
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
…without initializing the manager,
GetMarkerKey …WebRootComponentManager is initialized for dynamic JS roots
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.
Pull Request Overview
This PR fixes a bug where dynamically added JavaScript root components with persistent state were causing circuit errors because they were entering the GetMarkerKey method without the _webRootComponentManager being initialized. The fix ensures the manager is created if it doesn't exist when getting the marker key.
Key changes:
- Modified
GetMarkerKeymethod in bothRemoteRenderer.csandWebAssemblyRenderer.csto lazily initialize_webRootComponentManagerif null - Added test coverage with a new E2E test to verify JS-added persistent state root components work correctly
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/Components/Server/src/Circuits/RemoteRenderer.cs |
Fixed GetMarkerKey to call GetOrCreateWebRootComponentManager() when manager is null |
src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs |
Fixed GetMarkerKey to call GetOrCreateWebRootComponentManager() when manager is null |
src/Components/test/E2ETest/Tests/StatePersistenceTest.cs |
Added new test case for JS-added persistent state root components |
src/Components/test/testassets/BasicTestApp/wwwroot/persistent-state-js-root-component.html |
Added test HTML page for JS root component scenario |
src/Components/test/testassets/Components.TestServer/RazorComponentEndpointsStartup.cs |
Registered CounterWithPersistentState component for JavaScript testing |
src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/PersistentState/CounterWithPersistentState.razor |
Added test component with persistent state for testing |
...Components.TestServer/RazorComponents/Pages/PersistentState/CounterWithPersistentState.razor
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…onents/Pages/PersistentState/CounterWithPersistentState.razor Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…er is initialized on time.
src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs
Outdated
Show resolved
Hide resolved
|
The tests: The failure happens on reinitialization after shutdown. The reinitialization process doesn't properly handle the case where JS components with persistent state are registered but not yet added to the circuit. The circuit seems to get "stuck" during reinitialization - it connects (WebSocket shows connected): the HTML shows the component markup, but the components never become interactive ( The question is: how adding |
Calling
Edit: |
javiercn
left a comment
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.
Looks great!
Fixes #64159.
Source of the issue:
_webRootComponentManageris not defined inaspnetcore/src/Components/Server/src/Circuits/RemoteRenderer.cs
Line 324 in 10ef000
How it typically works :
_webRootComponentManagergets created whenCircuitHostgoes throughaspnetcore/src/Components/Server/src/Circuits/CircuitHost.cs
Line 882 in 10ef000
That is the normal flow for prerendered/SSR roots: the client sends an
UpdateRootComponentsmessage,CircuitHostmaterialises the operations, the manager is created, and every root component added in that batch is registered inside the manager’s dictionary.How it works with dynamic JS root:
Blazor.rootComponents.add(...). That call hitsCircuitJSComponentInterop.AddRootComponent, which simply callsbase.AddRootComponent(...)on the renderer:aspnetcore/src/Components/Web.JS/src/Rendering/JSRootComponents.ts
Line 27 in 6819df1
aspnetcore/src/Components/Server/src/Circuits/CircuitJSComponentInterop.cs
Line 26 in 10ef000
(
getRequiredManageris not the manager we are interested in, it's only a set of interop methods available for a given component, seeaspnetcore/src/Components/Web.JS/src/Rendering/WebRendererInteropMethods.ts
Line 78 in 6819df1
CircuitHost.PerformRootComponentOperationspipeline, so_webRootComponentManagernever gets initialised and no entry is recorded for the new component.RemoteComponentStatefor that JS-added root,RemoteComponentState.GetComponentKey()invokesRemoteRenderer.GetMarkerKey(...). Because the manager had never been created on this code path,_webRootComponentManagerwas still null and the null-forgiving access!resulted in theNullReferenceException.Fix: