[Blazor] Fix metadata comments incorrectly being processed as logical elements #64529
Conversation
|
/backport to release/10.0 |
|
Started backporting to |
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical Blazor navigation bug where metadata comments (WebAssembly options, component state, and initializers) were incorrectly being converted into logical elements during initial rendering. When these comments were later removed from the DOM during component discovery, they remained in the logical tree as orphaned references, causing insertBefore errors during subsequent navigation between different render modes.
Key changes:
- Added
isMetadataComment()helper function to identify all Blazor metadata comment types - Modified
toLogicalElement()to skip metadata comments during logical tree construction - Prevents orphaned references that cause navigation failures in Blazor Web Apps with mixed render modes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Components/Web.JS/src/Services/ComponentDescriptorDiscovery.ts | Adds isMetadataComment() function to identify metadata comments (Blazor-Server-Component-State, Blazor-WebAssembly-Component-State, Blazor-Web-Initializers, Blazor-WebAssembly) that should not become logical elements |
| src/Components/Web.JS/src/Rendering/LogicalElements.ts | Filters out metadata comments in toLogicalElement() to prevent them from entering the logical tree during initial render |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| return content.trim().startsWith('Blazor-Server-Component-State:') || | ||
| content.trim().startsWith('Blazor-WebAssembly-Component-State:') || | ||
| content.trim().startsWith('Blazor-Web-Initializers:') || | ||
| content.trim().startsWith('Blazor-WebAssembly:'); |
There was a problem hiding this comment.
The content.trim() call is repeated up to 4 times in the boolean expression. Store the trimmed content in a variable to avoid redundant string operations:
const content = node.textContent || '';
const trimmedContent = content.trim();
return trimmedContent.startsWith('Blazor-Server-Component-State:') ||
trimmedContent.startsWith('Blazor-WebAssembly-Component-State:') ||
trimmedContent.startsWith('Blazor-Web-Initializers:') ||
trimmedContent.startsWith('Blazor-WebAssembly:');| return content.trim().startsWith('Blazor-Server-Component-State:') || | |
| content.trim().startsWith('Blazor-WebAssembly-Component-State:') || | |
| content.trim().startsWith('Blazor-Web-Initializers:') || | |
| content.trim().startsWith('Blazor-WebAssembly:'); | |
| const trimmedContent = content.trim(); | |
| return trimmedContent.startsWith('Blazor-Server-Component-State:') || | |
| trimmedContent.startsWith('Blazor-WebAssembly-Component-State:') || | |
| trimmedContent.startsWith('Blazor-Web-Initializers:') || | |
| trimmedContent.startsWith('Blazor-WebAssembly:'); |
| export function isMetadataComment(node: Node): boolean { | ||
| if (node.nodeType !== Node.COMMENT_NODE) { | ||
| return false; | ||
| } | ||
| const content = node.textContent || ''; | ||
| return content.trim().startsWith('Blazor-Server-Component-State:') || | ||
| content.trim().startsWith('Blazor-WebAssembly-Component-State:') || | ||
| content.trim().startsWith('Blazor-Web-Initializers:') || | ||
| content.trim().startsWith('Blazor-WebAssembly:'); | ||
| } |
There was a problem hiding this comment.
The isMetadataComment function lacks automated test coverage. Given that this function is critical for preventing metadata comments from entering the logical tree (which causes navigation errors), consider adding unit tests to verify:
- Correct identification of all metadata comment types (Blazor-Server-Component-State, Blazor-WebAssembly-Component-State, Blazor-Web-Initializers, Blazor-WebAssembly)
- Rejection of non-comment nodes
- Rejection of non-metadata comments (e.g., regular Blazor: component markers)
- Handling of whitespace variations in comment content
Example test structure:
describe('isMetadataComment', () => {
test('should identify server component state comments', () => {
const comment = document.createComment('Blazor-Server-Component-State:abc123');
expect(isMetadataComment(comment)).toBe(true);
});
test('should not identify component marker comments', () => {
const comment = document.createComment('Blazor:{"type":"server"}');
expect(isMetadataComment(comment)).toBe(false);
});
// ... more tests
});| element.childNodes.forEach(child => { | ||
| // Skip metadata comments that will be consumed during discovery | ||
| // These are not components and should not be part of the logical tree | ||
| if (isMetadataComment(child)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The metadata comment filtering logic in toLogicalElement lacks automated test coverage. This is a critical fix for navigation errors, so consider adding tests to verify:
- Metadata comments are excluded from the logical tree during initial construction
- Non-metadata comments and other nodes are still included
- The logical tree structure remains correct after filtering
Example test:
test('should exclude metadata comments from logical tree', () => {
const parent = document.createElement('div');
parent.appendChild(document.createComment('Blazor-WebAssembly:{"options":"test"}'));
parent.appendChild(document.createElement('span'));
parent.appendChild(document.createComment('Regular comment'));
const logicalElement = toLogicalElement(parent, true);
const children = getLogicalChildrenArray(logicalElement);
expect(children.length).toBe(2); // span and regular comment, but not metadata comment
});
ilonatommy
left a comment
ilonatommy
left a comment
There was a problem hiding this comment.
+1 to copilot's review: tests missing. Other than that, approved.
I skipped the unit test because it doesn't prove anything. I considered adding an E2E test for this, but it will be complex to write. The issue is actually an edge case (it won't happen on all navigations, only when you have a component in a concrete DOM location) I did manual validation with two independent repros to confirm it fixed the issue. |
Fix Blazor navigation error caused by metadata comments in logical tree
Prevent metadata comments from becoming logical elements during rendering
Description
This PR fixes a bug where Blazor metadata comments (such as WebAssembly options, component state, and initializers) were being converted into logical elements during the initial render. When these comments were later removed from the DOM during component discovery, they remained in the logical tree as orphaned references, causing
insertBeforeerrors during subsequent navigation between different render modes.The fix modifies
toLogicalElement()inLogicalElements.tsto skip metadata comments before they enter the logical tree, using a newisMetadataComment()helper function inComponentDescriptorDiscovery.tsthat identifies all Blazor metadata comment types.Fixes #64522, #64472
Customer Impact
Users experience JavaScript errors and navigation failures when navigating between pages with different render modes (InteractiveServer, InteractiveWebAssembly, or InteractiveAuto) in Blazor Web Apps. The error manifests as:
Uncaught TypeError: Cannot read properties of null (reading 'insertBefore')This particularly affects applications using:
PageTitlecomponents with different render modesRegression?
This is a regression introduced in .NET 10.0 when the WebAssembly options discovery mechanism was added. The
<!--Blazor-WebAssembly:{...}-->comment is new to v10.0 and was not present in v9.0. ThetoLogicalElement()function was indiscriminately converting all nodes including these new metadata comments into logical elements, which caused the orphaning issue when the comments were removed from the DOM.Risk
Justification:
isMetadataComment()function uses explicit prefix matching with well-defined comment formatsVerification
Manual verification completed:
BlazorApp10 (Issue .Net 10 Blazor Enhanced Navigation in WASM fails due to no PageTitle #64522): Validated navigation between InteractiveServer pages with/without PageTitle components, and InteractiveWebAssembly pages. Multiple navigation cycles completed without errors.
BugReportApp (Issue .NET 10 Blazor navigation from wasm to interactive server broken #64472): Validated navigation between InteractiveWebAssembly → InteractiveServer → InteractiveWebAssembly. Both directions work correctly with SignalR connections established properly.
Packaging changes reviewed?
This is a JavaScript/TypeScript code change only. No packaging changes required.
When servicing release/2.3