fix(core): preserve leave animation for sibling instances sharing a TNode#69296
Open
kaiguogit wants to merge 1 commit into
Open
fix(core): preserve leave animation for sibling instances sharing a TNode#69296kaiguogit wants to merge 1 commit into
kaiguogit wants to merge 1 commit into
Conversation
…Node `animate.leave` was skipped — the element was removed from the DOM synchronously instead of running its leave animation — whenever a sibling instance of the same template entered in a different DOM parent during the same change-detection tick (e.g. an exclusive-expansion accordion or nav where opening section B collapses section A). `leavingNodes` is keyed by `TNode`, which is shared by every instance of a template. When a node was inserted, `cancelLeavingNodes` force-removed any tracked leaving node whose DOM parent differed from the entering node's parent (the `leavingParent !== newParent` branch added to de-duplicate a dynamic component re-rendered into a fresh overlay pane). For two distinct live sibling instances that merely share a `TNode`, "different parent" is the normal situation, so the still-animating sibling was ripped out. Track the declaration view of each leaving element alongside it, and only perform the cross-parent removal when the entering element belongs to the same declaration view as the leaving one — i.e. the same logical view re-rendered, the case the branch was written for. Two distinct instances of a shared template have different declaration views, so their `animate.leave` is now left to run to completion. This preserves the dynamic-component/overlay de-duplication (angular#67032) and the drag-and-drop node-move rescue (angular#67361), which are unchanged. Fixes angular#69291
thePunderWoman
approved these changes
Jun 10, 2026
thePunderWoman
left a comment
Contributor
There was a problem hiding this comment.
LGTM! Thanks for catching and fixing this!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Fixes #69291
Summary
animate.leaveis skipped — the element is removed from the DOM synchronously instead of running its leave animation — whenever a sibling instance of the same template enters in a different DOM parent during the same change-detection tick. This is the common "close section A while opening section B" pattern: accordions, tab groups, exclusive-expansion menus, master/detail navigation, where A and B are two instances of the same child component. It is a regression introduced in 21.2.0; it worked in 21.1.x.Root cause
leavingNodes(packages/core/src/animation/utils.ts) is keyed byTNode, which is part of the compiledTViewand is therefore shared by every instance of a template. So the animated element in instance A and the one in instance B map to the sameTNodebucket.On insert,
applyToElementOrContainercallscancelLeavingNodes(tNode, enteringEl). Its cross-parent branch:The
leavingParent !== newParentremoval was added in #67032 to de-duplicate a single dynamic component re-rendered into a fresh overlay/portal parent (e.g. CDK Overlay). But "different parent" is also the normal situation for two legitimate sibling instances, so it produces a false positive: A's still-animating element is force-removed because its host differs from B's host. (#67361 later rescued the node-move case vialeavingEl === newElement, but the cross-parent removal for distinct sibling elements was untouched.)The fix
The collision is purely a matter of tracking by
TNodewith no instance identity. We add instance identity:leavingNodesnow stores{el, declarationView}per leaving node instead of a bareHTMLElement.trackLeavingNodes(called from the Detach branch, which has the leaving node'sLView) records the declaration view (lView[DECLARATION_VIEW] ?? lView) of the leaving element.cancelLeavingNodes(called from the Insert branch, which has the entering node'sLView) resolves the entering node's declaration view and only performs the cross-parent removal when it matches the leaving node's declaration view — i.e. the same logical view re-rendered. Distinct instances of a shared template have different declaration views, so they are left alone and theiranimate.leaveruns to completion.The declaration view (rather than the rendered
LView, which is a fresh object on every re-render) is used so the same logical insertion point compares equal across re-renders while distinct instances compare unequal. When no owning view is known on either side, the original (conservative) removal behavior is kept.The
prevSibling(same-position@if/VCR toggle) branch and theleavingEl === newElement(drag-and-drop node move, #67361) branch are unchanged.Why this preserves #67032 and #67361
open()re-creates the embedded view from the sameViewContainerRefon the same component instance, so every embedded view'sDECLARATION_VIEWis the same componentLView.newDeclarationView === leavingDeclarationViewholds, so the cross-parent removal still fires and the duplicate is removed. Its acceptance test ("should not duplicate elements when using dynamic components in overlay-like containers") passes.leavingEl === newElementbranch, which is untouched.New regression test
Added to
packages/core/test/acceptance/animation_spec.tsin the existinganimation element duplicationsuite: "should run animate.leave for a sibling instance when another instance of the same template enters". It renders two instances of one component (panels sharing aTNode), closes A and opens B in the same tick, and asserts A's panel stays connected to the DOM (leave animation running) until itsanimationend, instead of being removed synchronously.I verified this test fails on
main("Expected false to be true" at theleavingPanelA.isConnectedassertion — A removed synchronously) and passes with the fix.Local verification
Built and tested with Bazel locally (Node 22.22.3, pnpm 11.5.2):
//packages/core/test/acceptance:acceptance_web_chromium— PASS (2658 passed incl. the new test; the existing fix(core): prevent animated element duplication with dynamic components in zoneless mode #67032 overlay-dedup test green)//packages/core/test/acceptance:acceptance_web_firefox— PASS//packages/core/test/acceptance:acceptance(Node/jasmine) — PASSgetDeclarationViewsurvives tree-shaking, so the 8 affectedbundle.golden_symbols.jsonfiles were updated via the:symbol_test.accepttargets; all 8symbol_tests pass.prettier --checkpasses on all changed source/test files.Not run locally: the full repository test matrix and the
integration/animationse2e harness (could not bring up the integration browser tooling in this environment). CI will cover those.Uncertainty for reviewers
getDeclarationView) to the animation-containing bundles. The golden updates are mechanical, but flagging the size delta explicitly.