-
Notifications
You must be signed in to change notification settings - Fork 0
Allow JS root components to reinitialize on circuit restart #7
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
base: coderabbit_full_base_allow_js_root_components_to_reinitialize_on_circuit_restart_pr7
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,7 +10,9 @@ let nextPendingDynamicRootComponentIdentifier = 0; | |||||||||||||||||||||||||||||||
| type ComponentParameters = object | null | undefined; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| let manager: DotNet.DotNetObject | undefined; | ||||||||||||||||||||||||||||||||
| let currentRendererId: number | undefined; | ||||||||||||||||||||||||||||||||
| let jsComponentParametersByIdentifier: JSComponentParametersByIdentifier; | ||||||||||||||||||||||||||||||||
| let hasInitializedJsComponents = false; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // These are the public APIs at Blazor.rootComponents.* | ||||||||||||||||||||||||||||||||
| export const RootComponentsFunctions = { | ||||||||||||||||||||||||||||||||
|
|
@@ -116,28 +118,35 @@ class DynamicRootComponent { | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Called by the framework | ||||||||||||||||||||||||||||||||
| export function enableJSRootComponents( | ||||||||||||||||||||||||||||||||
| rendererId: number, | ||||||||||||||||||||||||||||||||
| managerInstance: DotNet.DotNetObject, | ||||||||||||||||||||||||||||||||
| jsComponentParameters: JSComponentParametersByIdentifier, | ||||||||||||||||||||||||||||||||
| jsComponentInitializers: JSComponentIdentifiersByInitializer | ||||||||||||||||||||||||||||||||
| ): void { | ||||||||||||||||||||||||||||||||
| if (manager) { | ||||||||||||||||||||||||||||||||
| // This will only happen in very nonstandard cases where someone has multiple hosts. | ||||||||||||||||||||||||||||||||
| // It's up to the developer to ensure that only one of them enables dynamic root components. | ||||||||||||||||||||||||||||||||
| if (manager && currentRendererId === rendererId) { | ||||||||||||||||||||||||||||||||
| // A different renderer type (e.g., Server vs WebAssembly) is trying to enable JS root components. | ||||||||||||||||||||||||||||||||
| // This is a multi-host scenario which is not supported for dynamic root components. | ||||||||||||||||||||||||||||||||
| throw new Error('Dynamic root components have already been enabled.'); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // When the same renderer type re-enables (e.g., circuit restart or new circuit on same page), | ||||||||||||||||||||||||||||||||
| // accept the new manager. The old manager's DotNetObjectReference is no longer valid anyway | ||||||||||||||||||||||||||||||||
| // because the old circuit is gone. We don't dispose the old manager - doing so would cause | ||||||||||||||||||||||||||||||||
| // JSDisconnectedException because the circuit that created it no longer exists. | ||||||||||||||||||||||||||||||||
| currentRendererId = rendererId; | ||||||||||||||||||||||||||||||||
| manager = managerInstance; | ||||||||||||||||||||||||||||||||
| jsComponentParametersByIdentifier = jsComponentParameters; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Call the registered initializers. This is an arbitrary subset of the JS component types that are registered | ||||||||||||||||||||||||||||||||
| // on the .NET side - just those of them that require some JS-side initialization (e.g., to register them | ||||||||||||||||||||||||||||||||
| // as custom elements). | ||||||||||||||||||||||||||||||||
| for (const [initializerIdentifier, componentIdentifiers] of Object.entries(jsComponentInitializers)) { | ||||||||||||||||||||||||||||||||
| const initializerFunc = DotNet.findJSFunction(initializerIdentifier, 0) as JSComponentInitializerCallback; | ||||||||||||||||||||||||||||||||
| for (const componentIdentifier of componentIdentifiers) { | ||||||||||||||||||||||||||||||||
| const parameters = jsComponentParameters[componentIdentifier]; | ||||||||||||||||||||||||||||||||
| initializerFunc(componentIdentifier, parameters); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
Comment on lines
+136
to
+138
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refresh JS component parameter metadata on re-enable. 🛠️ Proposed fix currentRendererId = rendererId;
manager = managerInstance;
+ jsComponentParametersByIdentifier = jsComponentParameters;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| if (!hasInitializedJsComponents) { | ||||||||||||||||||||||||||||||||
| // Call the registered initializers. This is an arbitrary subset of the JS component types that are registered | ||||||||||||||||||||||||||||||||
| // on the .NET side - just those of them that require some JS-side initialization (e.g., to register them | ||||||||||||||||||||||||||||||||
| // as custom elements). | ||||||||||||||||||||||||||||||||
| for (const [initializerIdentifier, componentIdentifiers] of Object.entries(jsComponentInitializers)) { | ||||||||||||||||||||||||||||||||
| const initializerFunc = DotNet.findJSFunction(initializerIdentifier, 0) as JSComponentInitializerCallback; | ||||||||||||||||||||||||||||||||
| for (const componentIdentifier of componentIdentifiers) | ||||||||||||||||||||||||||||||||
| initializerFunc(componentIdentifier, jsComponentParameters[componentIdentifier]); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| hasInitializedJsComponents = true; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
This file was deleted.
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.
Fix renderer-id guard to allow same-renderer re-enable.
Line 126 currently throws when the same renderer re-attaches, which blocks circuit restart (the scenario this change targets). Flip the comparison to only throw on a different renderer id.
🔧 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents