Skip to content

Commit 3b7780b

Browse files
fix(core): debounce leave animation instruction calls
There are cases where the animation instructions get called in rapid succession. For example: ``` @if (show$ | async) { <div animate.leave="class-name">...</div> } ``` In this situation, the leave instruction may be called several times. With `animate.leave` this causes problems due to the promises that are awaited, but will never resolve. This results in duplicated or nodes that are left behind. This PR adds a debounce based on the lView, tNode index, and value. The main reason we need to key off of the value is because of host binding composition, which is a legitimate reason we may have multiple calls in rapid succession. We need to still allow those to happen. `animate.enter` would also be called in rapid succession in this same scenario, but there's no concerns with that situation since there's no promises present. It would resolve on its own. fixes: angular#64581
1 parent e47ef3e commit 3b7780b

File tree

4 files changed

+58
-17
lines changed

4 files changed

+58
-17
lines changed

packages/core/src/animation/interfaces.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,12 @@ export interface LongestAnimation {
7676
duration: number;
7777
}
7878

79+
export type AnimationValue = string | Function | AnimationFunction;
80+
7981
export interface NodeAnimations {
8082
animateFns: Function[];
8183
resolvers?: VoidFunction[];
84+
timeouts?: Map<AnimationValue, unknown>;
8285
}
8386

8487
export interface AnimationLViewData {

packages/core/src/animation/utils.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {stringify} from '../util/stringify'; // Adjust imports as per actual location
10-
import {ANIMATIONS_DISABLED, LongestAnimation, NodeAnimations} from './interfaces';
10+
import {ANIMATIONS_DISABLED, AnimationValue, LongestAnimation, NodeAnimations} from './interfaces';
1111
import {INJECTOR, LView, DECLARATION_LCONTAINER, ANIMATIONS} from '../render3/interfaces/view';
1212
import {RuntimeError, RuntimeErrorCode} from '../errors';
1313
import {Renderer} from '../render3/interfaces/renderer';
@@ -16,6 +16,7 @@ import {TNode} from '../render3/interfaces/node';
1616
import {getBeforeNodeForView} from '../render3/node_manipulation';
1717

1818
const DEFAULT_ANIMATIONS_DISABLED = false;
19+
const ANIMATION_LEAVE_DEBOUNCE_TIMEOUT = 10;
1920

2021
export const areAnimationSupported =
2122
(typeof ngServerMode === 'undefined' || !ngServerMode) &&
@@ -292,3 +293,29 @@ export function leaveAnimationFunctionCleanup(
292293
cleanupAfterLeaveAnimations(resolvers, cleanupFns);
293294
clearLViewNodeAnimationResolvers(lView, tNode);
294295
}
296+
297+
/**
298+
* We have to debounce leave animations due to potential for legitimate rapid calling of the instruction
299+
* in cases with async control flow values. Due to composition with host bindings, we need to key off
300+
* of the value, as composition with host bindings are a valid reason for rapid calls.
301+
*/
302+
export function debounceLeaveAnimation(
303+
lView: LView,
304+
tNode: TNode,
305+
value: AnimationValue,
306+
animateFn: Function,
307+
) {
308+
const animations = getLViewLeaveAnimations(lView);
309+
const nodeAnimations = animations.get(tNode.index) ?? {animateFns: []};
310+
if (nodeAnimations.timeouts && nodeAnimations.timeouts.has(value)) {
311+
clearTimeout(nodeAnimations.timeouts.get(value) as number);
312+
}
313+
const timeouts = (nodeAnimations.timeouts ??= new Map<AnimationValue, unknown>());
314+
const timeoutId = setTimeout(() => {
315+
timeouts.delete(value);
316+
animateFn();
317+
}, ANIMATION_LEAVE_DEBOUNCE_TIMEOUT);
318+
319+
timeouts.set(value, timeoutId);
320+
animations.set(tNode.index, nodeAnimations);
321+
}

packages/core/src/render3/instructions/animation.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
cleanupEnterClassData,
3434
clearLeavingNodes,
3535
clearLViewNodeAnimationResolvers,
36+
debounceLeaveAnimation,
3637
enterClassMap,
3738
getClassListFromValue,
3839
getLViewEnterAnimations,
@@ -251,11 +252,12 @@ export function ɵɵanimateLeave(value: string | Function): typeof ɵɵanimateLe
251252
const tNode = getCurrentTNode()!;
252253
cancelLeavingNodes(tNode, lView);
253254

254-
addAnimationToLView(getLViewLeaveAnimations(lView), tNode, () =>
255-
runLeaveAnimations(lView, tNode, value),
256-
);
257-
258-
initializeAnimationQueueScheduler(lView[INJECTOR]);
255+
debounceLeaveAnimation(lView, tNode, value, () => {
256+
addAnimationToLView(getLViewLeaveAnimations(lView), tNode, () =>
257+
runLeaveAnimations(lView, tNode, value),
258+
);
259+
initializeAnimationQueueScheduler(lView[INJECTOR]);
260+
});
259261

260262
return ɵɵanimateLeave; // For chaining
261263
}
@@ -341,6 +343,7 @@ function animateLeaveClassRunner(
341343
for (const item of classList) {
342344
renderer.addClass(el, item);
343345
}
346+
344347
// In the case that the classes added have no animations, we need to remove
345348
// the element right away. This could happen because someone is intentionally
346349
// preventing an animation via selector specificity.
@@ -384,13 +387,13 @@ export function ɵɵanimateLeaveListener(value: AnimationFunction): typeof ɵɵa
384387
const tNode = getCurrentTNode()!;
385388
cancelLeavingNodes(tNode, lView);
386389

387-
allLeavingAnimations.add(lView);
388-
389-
addAnimationToLView(getLViewLeaveAnimations(lView), tNode, () =>
390-
runLeaveAnimationFunction(lView, tNode, value),
391-
);
392-
393-
initializeAnimationQueueScheduler(lView[INJECTOR]);
390+
debounceLeaveAnimation(lView, tNode, value, () => {
391+
allLeavingAnimations.add(lView);
392+
addAnimationToLView(getLViewLeaveAnimations(lView), tNode, () =>
393+
runLeaveAnimationFunction(lView, tNode, value),
394+
);
395+
initializeAnimationQueueScheduler(lView[INJECTOR]);
396+
});
394397

395398
return ɵɵanimateLeaveListener; // For chaining
396399
}

packages/core/test/acceptance/animation_spec.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,7 +1651,7 @@ describe('Animation', () => {
16511651
encapsulation: ViewEncapsulation.None,
16521652
})
16531653
class TestComponent {
1654-
show = signal(false);
1654+
show = signal(true);
16551655
cdr = inject(ChangeDetectorRef);
16561656

16571657
toggle() {
@@ -1674,8 +1674,16 @@ describe('Animation', () => {
16741674
cmp.toggle();
16751675
fixture.detectChanges();
16761676
tickAnimationFrames(1);
1677-
const paragraphs = fixture.debugElement.queryAll(By.css('p'));
1678-
expect(paragraphs.length).toBe(1);
1677+
1678+
fixture.debugElement
1679+
.query(By.css('p'))
1680+
.nativeElement.dispatchEvent(
1681+
new AnimationEvent('animationend', {animationName: 'fade-out'}),
1682+
);
1683+
fixture.detectChanges();
1684+
tickAnimationFrames(1);
1685+
1686+
expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(0);
16791687
}));
16801688

16811689
it('should always run animations for `@for` loops when adding and removing quickly', fakeAsync(() => {
@@ -2045,7 +2053,6 @@ describe('Animation', () => {
20452053
cmp.removeSecondToLast();
20462054
fixture.detectChanges();
20472055
tickAnimationFrames(1);
2048-
20492056
expect(fixture.debugElement.queryAll(By.css('p.fade')).length).toBe(1);
20502057
expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(4);
20512058
fixture.debugElement
@@ -2219,6 +2226,7 @@ describe('Animation', () => {
22192226
const fixture = TestBed.createComponent(TestComponent);
22202227
const cmp = fixture.componentInstance;
22212228
fixture.detectChanges();
2229+
tick(10);
22222230

22232231
expect(fixture.debugElement.query(By.css('p.all-there-is'))).not.toBeNull();
22242232
expect(fixture.debugElement.query(By.css('p.not-here.fade-out'))).not.toBeNull();

0 commit comments

Comments
 (0)