Skip to content

Commit dfa149d

Browse files
thePunderWomanleonsenft
authored andcommitted
fix(core): fixes a regression with animate.leave and reordering
This fixes a regression bug that resulted in reordered elements not getting properly removed from the DOM. Reused nodes were not being cleared out in this situation. fixes: #67728
1 parent 41b1410 commit dfa149d

File tree

5 files changed

+99
-1
lines changed

5 files changed

+99
-1
lines changed

integration/animations/e2e/src/animations.e2e-spec.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,23 @@ describe('Animations Integration', () => {
114114
'Nested child component should have been removed immediately during routing',
115115
);
116116
});
117+
118+
it('should remove elements from DOM after reordering and removal with (animate.leave)', async () => {
119+
// Wait for the test elements to be rendered
120+
await page.waitForSelector('.test-item');
121+
122+
let items = await page.$$('.test-item');
123+
expect(items.length).toBe(2);
124+
125+
// Shuffle
126+
await page.click('#shuffle-test');
127+
await new Promise((res) => setTimeout(res, 500));
128+
129+
// Remove
130+
await page.click('#remove-test');
131+
await new Promise((res) => setTimeout(res, 500));
132+
133+
items = await page.$$('.test-item');
134+
expect(items.length).toBe(1);
135+
});
117136
});

integration/animations/src/app/app.component.html

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,11 @@
1717
}
1818

1919
<button id="hide-and-intercept" (click)="hideAndIntercept()">Hide and Intercept</button>
20+
21+
<div class="test-reorder-container">
22+
@for (item of testItems; track item) {
23+
<div class="test-item" (animate.leave)="onTestLeave($event)">Item {{ item }}</div>
24+
}
25+
</div>
26+
<button id="shuffle-test" (click)="shuffleTest()">Shuffle Test</button>
27+
<button id="remove-test" (click)="removeTest()">Remove Test</button>

integration/animations/src/app/home.component.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Component} from '@angular/core';
1+
import {Component, AnimationCallbackEvent} from '@angular/core';
22
import {
33
CdkDragDrop,
44
CdkDropList,
@@ -20,6 +20,8 @@ export class HomeComponent {
2020
'Episode III - Revenge of the Sith',
2121
];
2222

23+
testItems = ['A', 'B'];
24+
2325
showFallback = true;
2426

2527
drop(event: CdkDragDrop<string[]>) {
@@ -39,4 +41,16 @@ export class HomeComponent {
3941
}
4042
this.showFallback = false;
4143
}
44+
45+
shuffleTest() {
46+
this.testItems = ['B', 'A'];
47+
}
48+
49+
removeTest() {
50+
this.testItems = ['A'];
51+
}
52+
53+
onTestLeave(event: AnimationCallbackEvent) {
54+
event.animationComplete();
55+
}
4256
}

packages/core/src/render3/node_manipulation.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ function applyToElementOrContainer(
150150
if (parentLView?.[ANIMATIONS]?.leave?.has(tNode.index)) {
151151
trackLeavingNodes(tNode, rNode as HTMLElement);
152152
}
153+
reusedNodes.delete(rNode as HTMLElement);
153154
runLeaveAnimationsWithCallback(
154155
parentLView,
155156
tNode,
@@ -166,6 +167,7 @@ function applyToElementOrContainer(
166167
},
167168
);
168169
} else if (action === WalkTNodeTreeAction.Destroy) {
170+
reusedNodes.delete(rNode as HTMLElement);
169171
runLeaveAnimationsWithCallback(parentLView, tNode, injector, () => {
170172
renderer.destroyNode!(rNode);
171173
});

packages/core/test/acceptance/animation_spec.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {tickAnimationFrames} from '../animation_utils/tick_animation_frames';
3838
import {BrowserTestingModule, platformBrowserTesting} from '@angular/platform-browser/testing';
3939
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
4040
import {ComponentRef} from '@angular/core/src/render3';
41+
import {reusedNodes} from '../../src/animation/utils';
4142

4243
@NgModule({
4344
providers: [provideZonelessChangeDetection()],
@@ -511,6 +512,60 @@ describe('Animation', () => {
511512
expect(fadeCalled).toHaveBeenCalled();
512513
}));
513514

515+
it('should remove element from DOM with (animate.leave) after list reordering', async () => {
516+
@Component({
517+
selector: 'leak-cmp',
518+
template: 'item',
519+
})
520+
class LeakComponent {}
521+
522+
@Component({
523+
selector: 'test-cmp',
524+
imports: [LeakComponent],
525+
template: `
526+
@for (item of items(); track item.id) {
527+
<leak-cmp (animate.leave)="onLeave($event)" />
528+
}
529+
`,
530+
})
531+
class TestComponent {
532+
items = signal([{id: '1'}, {id: '2'}]);
533+
534+
onLeave(event: AnimationCallbackEvent) {
535+
event.animationComplete();
536+
}
537+
538+
shuffle() {
539+
const arr = this.items();
540+
this.items.set([arr[1], arr[0]]);
541+
}
542+
543+
remove() {
544+
const arr = this.items();
545+
this.items.set([arr[1]]);
546+
}
547+
}
548+
549+
TestBed.configureTestingModule({animationsEnabled: true});
550+
const fixture = TestBed.createComponent(TestComponent);
551+
await fixture.whenStable();
552+
553+
expect(fixture.nativeElement.textContent).toContain('item');
554+
555+
fixture.componentInstance.shuffle();
556+
await fixture.whenStable();
557+
558+
const elementsBeforeRemove = fixture.debugElement.queryAll(By.css('leak-cmp'));
559+
// Element is in reused nodes before remove
560+
expect(reusedNodes.has(elementsBeforeRemove[0].nativeElement)).toBe(true);
561+
562+
fixture.componentInstance.remove();
563+
await fixture.whenStable();
564+
565+
const elements = fixture.nativeElement.querySelectorAll('leak-cmp');
566+
expect(elements.length).toBe(1);
567+
});
568+
514569
it('should compose class list when host binding and regular binding', fakeAsync(() => {
515570
const multiple = `
516571
.slide-out {

0 commit comments

Comments
 (0)