mirror of
https://github.com/angular/angular.git
synced 2026-09-14 13:54:52 +08:00
refactor(core): Ensure determineLongestAnimation is run synchronously after style applies
This adds a setTimeout, which guarantees that we call getAnimations one frame after a reflow is finished. This means getAnimations will return data, avoiding needing the expensive fallback of getComputedStyles. It also updates the cleanup to prevent a potential memory leak if the component is destroyed before the timeout runs.
This commit is contained in:
committed by
Matthew Beck (Berry)
parent
df659b8d0c
commit
318ade062e
@@ -136,6 +136,11 @@ export function runEnterAnimation(
|
||||
|
||||
// We only need to add these event listeners if there are actual classes to apply
|
||||
if (activeClasses && activeClasses.length > 0) {
|
||||
let isCleanedUp = false;
|
||||
cleanupFns.push(() => {
|
||||
isCleanedUp = true;
|
||||
});
|
||||
|
||||
ngZone.runOutsideAngular(() => {
|
||||
cleanupFns.push(renderer.listen(nativeElement, 'animationstart', handleEnterAnimationStart));
|
||||
cleanupFns.push(renderer.listen(nativeElement, 'transitionstart', handleEnterAnimationStart));
|
||||
@@ -152,14 +157,16 @@ export function runEnterAnimation(
|
||||
// preventing an animation via selector specificity.
|
||||
ngZone.runOutsideAngular(() => {
|
||||
requestAnimationFrame(() => {
|
||||
if (hasCompleted) return;
|
||||
determineLongestAnimation(nativeElement, longestAnimations, areAnimationSupported);
|
||||
if (!longestAnimations.has(nativeElement)) {
|
||||
for (const klass of activeClasses) {
|
||||
renderer.removeClass(nativeElement, klass);
|
||||
setTimeout(() => {
|
||||
if (hasCompleted || isCleanedUp) return;
|
||||
determineLongestAnimation(nativeElement, longestAnimations, areAnimationSupported);
|
||||
if (!longestAnimations.has(nativeElement)) {
|
||||
for (const klass of activeClasses) {
|
||||
renderer.removeClass(nativeElement, klass);
|
||||
}
|
||||
cleanupEnterClassData(nativeElement);
|
||||
}
|
||||
cleanupEnterClassData(nativeElement);
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
@@ -326,6 +333,14 @@ function animateLeaveClassRunner(
|
||||
const componentResolvers = getLViewLeaveAnimations(lView).get(tNode.index)?.resolvers;
|
||||
let fallbackTimeoutId: number | undefined;
|
||||
let hasCompleted = false;
|
||||
let isCleanedUp = false;
|
||||
|
||||
cleanupFns.push(() => {
|
||||
isCleanedUp = true;
|
||||
if (fallbackTimeoutId !== undefined) {
|
||||
clearTimeout(fallbackTimeoutId);
|
||||
}
|
||||
});
|
||||
|
||||
const handleOutAnimationEnd = (event: AnimationEvent | TransitionEvent | CustomEvent) => {
|
||||
const target = getEventTarget(event as Event);
|
||||
@@ -378,21 +393,22 @@ function animateLeaveClassRunner(
|
||||
// preventing an animation via selector specificity.
|
||||
ngZone.runOutsideAngular(() => {
|
||||
requestAnimationFrame(() => {
|
||||
if (hasCompleted) return;
|
||||
determineLongestAnimation(el, longestAnimations, areAnimationSupported);
|
||||
const longest = longestAnimations.get(el);
|
||||
if (!longest) {
|
||||
clearLeavingNodes(tNode, el);
|
||||
cleanupAfterLeaveAnimations(componentResolvers, cleanupFns);
|
||||
clearLViewNodeAnimationResolvers(lView, tNode);
|
||||
} else {
|
||||
// Fallback cleanup if the browser drops the transitionend/animationend event
|
||||
// entirely due to off-screen optimizations or rapid DOM teardown.
|
||||
fallbackTimeoutId = setTimeout(() => {
|
||||
handleOutAnimationEnd(new CustomEvent('animation-fallback'));
|
||||
}, longest.duration + 50) as unknown as number;
|
||||
cleanupFns.push(() => clearTimeout(fallbackTimeoutId));
|
||||
}
|
||||
setTimeout(() => {
|
||||
if (hasCompleted || isCleanedUp) return;
|
||||
determineLongestAnimation(el, longestAnimations, areAnimationSupported);
|
||||
const longest = longestAnimations.get(el);
|
||||
if (!longest) {
|
||||
clearLeavingNodes(tNode, el);
|
||||
cleanupAfterLeaveAnimations(componentResolvers, cleanupFns);
|
||||
clearLViewNodeAnimationResolvers(lView, tNode);
|
||||
} else {
|
||||
// Fallback cleanup if the browser drops the transitionend/animationend event
|
||||
// entirely due to off-screen optimizations or rapid DOM teardown.
|
||||
fallbackTimeoutId = setTimeout(() => {
|
||||
handleOutAnimationEnd(new CustomEvent('animation-fallback'));
|
||||
}, longest.duration + 50) as unknown as number;
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -31,7 +31,7 @@ import {
|
||||
ViewChild,
|
||||
ViewContainerRef,
|
||||
} from '@angular/core';
|
||||
import {fakeAsync, TestBed, tick} from '@angular/core/testing';
|
||||
import {fakeAsync, flush, TestBed, tick} from '@angular/core/testing';
|
||||
import {By} from '@angular/platform-browser';
|
||||
import {isNode} from '@angular/private/testing';
|
||||
import {tickAnimationFrames} from '../animation_utils/tick_animation_frames';
|
||||
@@ -171,6 +171,68 @@ describe('Animation', () => {
|
||||
expect(cmp.el).toBeUndefined();
|
||||
}));
|
||||
|
||||
it('should fire the fallback timer and clean up if the animationend event is not dispatched', fakeAsync(() => {
|
||||
@Component({
|
||||
selector: 'test-cmp',
|
||||
styles: styles,
|
||||
template: '<div>@if (show()) {<p animate.leave="fade" #el>I should fade</p>}</div>',
|
||||
encapsulation: ViewEncapsulation.None,
|
||||
})
|
||||
class TestComponent {
|
||||
show = signal(true);
|
||||
@ViewChild('el', {read: ElementRef}) el!: ElementRef<HTMLParagraphElement>;
|
||||
}
|
||||
TestBed.configureTestingModule({animationsEnabled: true});
|
||||
|
||||
const fixture = TestBed.createComponent(TestComponent);
|
||||
const cmp = fixture.componentInstance;
|
||||
fixture.detectChanges();
|
||||
|
||||
expect(fixture.nativeElement.innerHTML).toContain('I should fade');
|
||||
cmp.show.set(false);
|
||||
fixture.detectChanges();
|
||||
|
||||
tickAnimationFrames(1);
|
||||
|
||||
// Now we step the tick so the macro-task setup timeout executes and queues our fallback.
|
||||
tick(1);
|
||||
|
||||
// Wait for duration + 50ms buffer time for fallback
|
||||
tick(10 + 50);
|
||||
|
||||
// Assert that cleanup occurred successfully without needing to artificially dispatch the event
|
||||
expect(fixture.nativeElement.innerHTML).not.toContain('I should fade');
|
||||
expect(cmp.el).toBeUndefined();
|
||||
}));
|
||||
|
||||
it('should prevent leaking fallback timeout if view is destroyed early', fakeAsync(() => {
|
||||
@Component({
|
||||
selector: 'test-cmp',
|
||||
styles: styles,
|
||||
template: '<div>@if (show()) {<p animate.leave="fade" #el>I should fade</p>}</div>',
|
||||
encapsulation: ViewEncapsulation.None,
|
||||
})
|
||||
class TestComponent {
|
||||
show = signal(true);
|
||||
@ViewChild('el', {read: ElementRef}) el!: ElementRef<HTMLParagraphElement>;
|
||||
}
|
||||
TestBed.configureTestingModule({animationsEnabled: true});
|
||||
|
||||
const fixture = TestBed.createComponent(TestComponent);
|
||||
const cmp = fixture.componentInstance;
|
||||
fixture.detectChanges();
|
||||
|
||||
cmp.show.set(false);
|
||||
fixture.detectChanges();
|
||||
|
||||
// We explicitly destroy the fixture right after triggering the leave,
|
||||
// imitating the situation where the view goes away before the next macro-task.
|
||||
fixture.destroy();
|
||||
|
||||
// Flush all tasks, expecting no errors with our cleanup flag guarding the timeout.
|
||||
flush();
|
||||
}));
|
||||
|
||||
it('should support string arrays', fakeAsync(() => {
|
||||
const multiple = `
|
||||
.slide-out {
|
||||
|
||||
Reference in New Issue
Block a user