Skip to content

fix(runtime): do not remove first comment - can break frameworks #6343

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

Merged
merged 2 commits into from
Jul 15, 2025

Conversation

johnjenkins
Copy link
Contributor

@johnjenkins johnjenkins commented Jul 15, 2025

What is the current behavior?

A regression was added in #6311.

In an attempt to 'clean up' non-shadow dom trees and minimise 3rd-party-framework hydration mismatch errors, the first empty comment node of a component was removed when the component does not utilise <slot />s.

This node removal had a side-effect as Angular uses empty comments as content placeholders (only in production builds) which caused the Angular app content to not render.

GitHub Issue Number: #6336)

What is the new behavior?

The node removal was reverted. I think with #6314 now in-place all non-shadow components correctly shield their internals from the outside, so this behaviour is no longer required anyway

Documentation

Does this introduce a breaking change?

  • Yes
  • No

Testing

Other information

@johnjenkins johnjenkins requested a review from a team as a code owner July 15, 2025 11:25
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

The Ionic pipeline is passing with this patch 🎉

@johnjenkins thank you so much, you truly have some magic DOM-power!

@christian-bromann christian-bromann merged commit 188e7db into main Jul 15, 2025
74 checks passed
@christian-bromann christian-bromann deleted the fix-runtime-comment-remove branch July 15, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants