Skip to content

fix(core): include inner ViewContainerRef anchor nodes into ViewRef.rootNodes output#49867

Closed
AndrewKushnir wants to merge 1 commit intoangular:mainfrom
AndrewKushnir:root-nodes-fix
Closed

fix(core): include inner ViewContainerRef anchor nodes into ViewRef.rootNodes output#49867
AndrewKushnir wants to merge 1 commit intoangular:mainfrom
AndrewKushnir:root-nodes-fix

Conversation

@AndrewKushnir
Copy link
Copy Markdown
Contributor

@AndrewKushnir AndrewKushnir commented Apr 15, 2023

Currently, the ViewRef.rootNodes output is missing anchor (comment) nodes for inner ViewContainerRefs, when an anchor node was created for that instance of a ViewContainerRef (which happens in all cases except when an was used as a host for a view container).

This issue affects hydration logic, which relies on the number of root nodes within a view to properly determine segments in DOM that belong to a particular view.

Resolves #49849.

PR Type

What kind of change does this PR introduce?

  • Bugfix

Does this PR introduce a breaking change?

  • Yes
  • No

@AndrewKushnir
fix(core): include inner ViewContainerRef anchor nodes into ViewRef.r…
8af03df 
…ootNodes output

Currently, the `ViewRef.rootNodes` output is missing anchor (comment) nodes for inner `ViewContainerRef`s,
when an achor node was created for that instance of a `ViewContainerRef` (which happens in all cases except
when an <ng-container> was used as a host for a view container).

This issue affects hydration logic, which relies on the number of root nodes within a view to properly determine
segments in DOM that belong to a particular view.

Resolves angular#49849.
@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: rc This PR is targeted for the next release-candidate labels Apr 15, 2023
@ngbot ngbot bot modified the milestone: Backlog Apr 15, 2023
@AndrewKushnir AndrewKushnir mentioned this pull request Apr 15, 2023
Closed
@AndrewKushnir
Copy link
Copy Markdown
Contributor Author

AndrewKushnir commented Apr 15, 2023

TGP.

@AndrewKushnir AndrewKushnir added the action: global presubmit The PR is in need of a google3 global presubmit label Apr 15, 2023
jessicajaniuk
jessicajaniuk approved these changes Apr 17, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

LGTM

@jessicajaniuk jessicajaniuk added action: merge The PR is ready for merge by the caretaker and removed action: global presubmit The PR is in need of a google3 global presubmit labels Apr 17, 2023
@ngbot
Copy link
Copy Markdown

ngbot bot commented Apr 17, 2023

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "google-internal-tests" is failing
    pending forbidden labels detected: action: review
    pending status "mergeability" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@jessicajaniuk jessicajaniuk added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Apr 17, 2023
@jessicajaniuk
Copy link
Copy Markdown
Contributor

jessicajaniuk commented Apr 17, 2023

Caretaker: TGP is green. Safe to merge.

@jessicajaniuk
Copy link
Copy Markdown
Contributor

jessicajaniuk commented Apr 17, 2023

This PR was merged into the repository by commit d994f85.

jessicajaniuk pushed a commit that referenced this pull request Apr 17, 2023
@AndrewKushnir @thePunderWoman
fix(core): include inner ViewContainerRef anchor nodes into ViewRef.r…
fedc756 
…ootNodes output (#49867)

Currently, the `ViewRef.rootNodes` output is missing anchor (comment) nodes for inner `ViewContainerRef`s,
when an achor node was created for that instance of a `ViewContainerRef` (which happens in all cases except
when an <ng-container> was used as a host for a view container).

This issue affects hydration logic, which relies on the number of root nodes within a view to properly determine
segments in DOM that belong to a particular view.

Resolves #49849.

PR Close #49867
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented May 18, 2023

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 May 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@pkozlowski-opensource pkozlowski-opensource pkozlowski-opensource approved these changes

+1 more reviewer

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: rc This PR is targeted for the next release-candidate

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

Rehydration doesn't work with throw error el.setAttribute is not a function in Angular 16 rc.1

3 participants

@AndrewKushnir @jessicajaniuk @pkozlowski-opensource