Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/Components/Web.JS/src/Rendering/LogicalElements.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

import { ComponentDescriptor } from '../Services/ComponentDescriptorDiscovery';
import { ComponentDescriptor, isMetadataComment } from '../Services/ComponentDescriptorDiscovery';

/*
A LogicalElement plays the same role as an Element instance from the point of view of the
Expand Down Expand Up @@ -109,6 +109,12 @@ export function toLogicalElement(element: Node, allowExistingContents?: boolean)
}

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;
}
Comment on lines 111 to +116
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Metadata comments are excluded from the logical tree during initial construction
  2. Non-metadata comments and other nodes are still included
  3. 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
});

Copilot uses AI. Check for mistakes.

const childLogicalElement = toLogicalElement(child, /* allowExistingContents */ true);
childLogicalElement[logicalParentPropname] = element;
childrenArray.push(childLogicalElement);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

// Metadata comments are consumed during discovery and should not be part of the logical tree.
// They include: WebAssembly options, component state, and web initializers.
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:');
Comment on lines +11 to +14
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:');
Suggested change
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:');

Copilot uses AI. Check for mistakes.
}
Comment on lines +6 to +15
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Correct identification of all metadata comment types (Blazor-Server-Component-State, Blazor-WebAssembly-Component-State, Blazor-Web-Initializers, Blazor-WebAssembly)
  2. Rejection of non-comment nodes
  3. Rejection of non-metadata comments (e.g., regular Blazor: component markers)
  4. 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
});

Copilot uses AI. Check for mistakes.

export function discoverComponents(root: Node, type: 'webassembly' | 'server' | 'auto'): ComponentDescriptor[] {
switch (type) {
case 'webassembly':
Expand Down
Loading