Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@hybrist
Copy link
Contributor

@hybrist hybrist commented Mar 18, 2025

When the browser parses a valid html5 response like this:

<!-- ... -->
<title>My page</title>
</head>
<!--nghm-->
<app-root></app-root>
<!-- ... -->

The resulting DOM will only start adding nodes to the body when it runs into the first non-header tag. E.g.:

- head
  - title "My page"
- comment "nghm"
- body
  - app-root

This isn't a sign that comments are modified, so it seems worth to handle it gracefully.

Motivation here is to allow some more minification of the HTML response and easier integration with systems that compose response fragments from different sources.

@angular-robot angular-robot bot added the area: core Issues related to the framework runtime label Mar 18, 2025
@ngbot ngbot bot added this to the Backlog milestone Mar 18, 2025
@hybrist hybrist changed the title fix: catch hydration marker with implicit body tag fix(core): catch hydration marker with implicit body tag Mar 18, 2025
@hybrist hybrist force-pushed the implicit-body branch 2 times, most recently from 08b9dd4 to 5b23cf6 Compare March 18, 2025 14:16
@hybrist hybrist marked this pull request as ready for review March 18, 2025 14:20
@pullapprove pullapprove bot requested a review from crisbeto March 18, 2025 14:20
@hybrist
Copy link
Contributor Author

hybrist commented Mar 18, 2025

Adding some tests because there's a few more edge cases I should handle here.

@AndrewKushnir AndrewKushnir self-requested a review March 18, 2025 15:07
@hybrist
Copy link
Contributor Author

hybrist commented Mar 18, 2025

Alright, this should be ready for some eyes. Now handles both the optional </head> and the optional <body> as well as text nodes that may be inserted if there's whitespace before the first genuine node/element of an implicit body.

packages/core/test/hydration/marker_spec.ts Outdated Show resolved Hide resolved
When the browser parses a valid html5 response like this:

```html
<!-- ... -->
<title>My page</title>
</head>
<!--nghm-->
<app-root></app-root>
<!-- ... -->
```

The resulting DOM will only start adding nodes to the body when it
runs into the first non-header tag. E.g.:

```yml
- head
  - title "My page"
- comment "nghm"
- body
  - app-root
```

This isn't a sign that comments are modified, so it seems worth to
handle it gracefully.
@hybrist hybrist added target: patch This PR is targeted for the next patch release action: merge The PR is ready for merge by the caretaker labels Mar 19, 2025
@pkozlowski-opensource pkozlowski-opensource removed the request for review from crisbeto March 19, 2025 09:10
@pkozlowski-opensource
Copy link
Member

This PR was merged into the repository by commit b461e06.

The changes were merged into the following branches: main, 19.2.x

pkozlowski-opensource pushed a commit that referenced this pull request Mar 19, 2025
When the browser parses a valid html5 response like this:

```html
<!-- ... -->
<title>My page</title>
</head>
<!--nghm-->
<app-root></app-root>
<!-- ... -->
```

The resulting DOM will only start adding nodes to the body when it
runs into the first non-header tag. E.g.:

```yml
- head
  - title "My page"
- comment "nghm"
- body
  - app-root
```

This isn't a sign that comments are modified, so it seems worth to
handle it gracefully.

PR Close #60429
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.