Skip to content

Conversation

@StephaneDelcroix
Copy link
Contributor

that was a premature optimization 10 years ago, revert that while we don't have too many conflicting branches

Copilot AI review requested due to automatic review settings September 10, 2025 21:12
@StephaneDelcroix StephaneDelcroix requested a review from a team as a code owner September 10, 2025 21:12
Copy link
Contributor

Copilot AI left a 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 pull request removes the IElementNode interface which was identified as a premature optimization from 10 years ago. The goal is to revert this abstraction while there aren't too many conflicting branches, simplifying the codebase by directly using the ElementNode class instead of the interface.

  • Removes the IElementNode interface entirely from the XAML node hierarchy
  • Updates all references to use ElementNode directly instead of IElementNode
  • Modernizes code with C# language features (primary constructors, collection expressions, pattern matching improvements)

Reviewed Changes

Copilot reviewed 61 out of 61 changed files in this pull request and generated 1 comment.

File Description
XamlNode.cs Removes IElementNode interface, updates ElementNode to implement interfaces directly
XmlName.cs Converts to modern C# syntax with readonly struct and primary constructor
Multiple visitor files Updates type references from IElementNode to ElementNode throughout the codebase
Test files Removes mock IElementNode implementations that are no longer needed

Comment on lines 42 to 51
#if NETSTANDARD2_0
if (NamespaceURI != null)
hashCode = NamespaceURI.GetHashCode();
if (LocalName != null)
hashCode = (hashCode * 397) ^ LocalName.GetHashCode();
int hashCode = 0;
if (NamespaceURI != null)
hashCode = NamespaceURI.GetHashCode();
if (LocalName != null)
hashCode = (hashCode * 397) ^ LocalName.GetHashCode();
return hashCode;
#else
if (NamespaceURI != null)
hashCode = NamespaceURI.GetHashCode(StringComparison.Ordinal);
if (LocalName != null)
hashCode = (hashCode * 397) ^ LocalName.GetHashCode(StringComparison.Ordinal);
return HashCode.Combine(NamespaceURI, LocalName);
#endif
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The NETSTANDARD2_0 conditional compilation removes the use of StringComparison.Ordinal that was present in the original code. This could lead to culture-sensitive hash code generation on .NET Standard 2.0, potentially causing issues with hash table operations across different cultures.

Copilot uses AI. Check for mistakes.
simonrozsival
simonrozsival previously approved these changes Sep 11, 2025
that was a premature optimization 10 years ago, revert that while we
don't have too many conflicting branches
@StephaneDelcroix StephaneDelcroix force-pushed the dev/stdelc/hk_removeIElementNode branch from 6888f5d to 4e76f37 Compare September 15, 2025 20:36
Co-authored-by: Šimon Rozsíval <simon@rozsival.com>
StephaneDelcroix and others added 2 commits September 15, 2025 22:38
Co-authored-by: Šimon Rozsíval <simon@rozsival.com>
Co-authored-by: MartyIX <203266+MartyIX@users.noreply.github.com>
@StephaneDelcroix StephaneDelcroix merged commit b8b8407 into net10.0 Sep 16, 2025
148 checks passed
@StephaneDelcroix StephaneDelcroix deleted the dev/stdelc/hk_removeIElementNode branch September 16, 2025 11:17
@github-actions github-actions bot locked and limited conversation to collaborators Oct 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants