From 308e5db7ef1c2ff16bc1bef7cb114d2b1ac32959 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Wed, 15 Oct 2025 08:54:18 -0700 Subject: [PATCH] fix(core): update animation scheduling In some rare cases, it seems the animation queue disappears despite being afterEveryRender. This updates the animation scheduler to be afterNextRender instead and only schedules it when we need to. fixes: #64423 --- packages/core/src/animation/interfaces.ts | 21 --- packages/core/src/animation/queue.ts | 79 ++++++++++++ .../src/render3/instructions/animation.ts | 57 ++------ .../core/src/render3/node_manipulation.ts | 18 +-- .../core/test/acceptance/animation_spec.ts | 122 ++++++++++++++++++ .../acceptance/view_container_ref_spec.ts | 2 +- .../bundle.golden_symbols.json | 1 + .../bundling/defer/bundle.golden_symbols.json | 1 + .../forms_reactive/bundle.golden_symbols.json | 1 + .../bundle.golden_symbols.json | 1 + .../hydration/bundle.golden_symbols.json | 1 + .../router/bundle.golden_symbols.json | 1 + .../bundle.golden_symbols.json | 1 + 13 files changed, 225 insertions(+), 81 deletions(-) create mode 100644 packages/core/src/animation/queue.ts diff --git a/packages/core/src/animation/interfaces.ts b/packages/core/src/animation/interfaces.ts index 52d40a322964..a8b4fbb0489d 100644 --- a/packages/core/src/animation/interfaces.ts +++ b/packages/core/src/animation/interfaces.ts @@ -18,27 +18,6 @@ export const ANIMATIONS_DISABLED = new InjectionToken( }, ); -export interface AnimationQueue { - queue: Set; - isScheduled: boolean; -} - -/** - * A [DI token](api/core/InjectionToken) for the queue of all animations. - */ -export const ANIMATION_QUEUE = new InjectionToken( - typeof ngDevMode !== 'undefined' && ngDevMode ? 'AnimationQueue' : '', - { - providedIn: 'root', - factory: () => { - return { - queue: new Set(), - isScheduled: false, - }; - }, - }, -); - /** * The event type for when `animate.enter` and `animate.leave` are used with function * callbacks. diff --git a/packages/core/src/animation/queue.ts b/packages/core/src/animation/queue.ts new file mode 100644 index 000000000000..4bc53cf12067 --- /dev/null +++ b/packages/core/src/animation/queue.ts @@ -0,0 +1,79 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.dev/license + */ + +import {afterNextRender} from '../render3/after_render/hooks'; +import {InjectionToken, Injector} from '../di'; +import {NodeAnimations} from './interfaces'; + +export interface AnimationQueue { + queue: Set; + isScheduled: boolean; + scheduler: Function | null; +} + +/** + * A [DI token](api/core/InjectionToken) for the queue of all animations. + */ +export const ANIMATION_QUEUE = new InjectionToken( + typeof ngDevMode !== 'undefined' && ngDevMode ? 'AnimationQueue' : '', + { + providedIn: 'root', + factory: () => { + return { + queue: new Set(), + isScheduled: false, + scheduler: null, + }; + }, + }, +); + +export function addToAnimationQueue(injector: Injector, animationFns: Function | Function[]) { + const animationQueue = injector.get(ANIMATION_QUEUE); + if (Array.isArray(animationFns)) { + for (const animateFn of animationFns) { + animationQueue.queue.add(animateFn); + } + } else { + animationQueue.queue.add(animationFns); + } + animationQueue.scheduler && animationQueue.scheduler(injector); +} + +export function scheduleAnimationQueue(injector: Injector) { + const animationQueue = injector.get(ANIMATION_QUEUE); + // We only want to schedule the animation queue if it hasn't already been scheduled. + if (!animationQueue.isScheduled) { + afterNextRender( + () => { + animationQueue.isScheduled = false; + for (let animateFn of animationQueue.queue) { + animateFn(); + } + animationQueue.queue.clear(); + }, + {injector}, + ); + animationQueue.isScheduled = true; + } +} + +export function initializeAnimationQueueScheduler(injector: Injector) { + const animationQueue = injector.get(ANIMATION_QUEUE); + animationQueue.scheduler = scheduleAnimationQueue; + animationQueue.scheduler(injector); +} + +export function queueEnterAnimations( + injector: Injector, + enterAnimations: Map, +) { + for (const [_, nodeAnimations] of enterAnimations) { + addToAnimationQueue(injector, nodeAnimations.animateFns); + } +} diff --git a/packages/core/src/render3/instructions/animation.ts b/packages/core/src/render3/instructions/animation.ts index db70efa4946e..916f60493fe5 100644 --- a/packages/core/src/render3/instructions/animation.ts +++ b/packages/core/src/render3/instructions/animation.ts @@ -7,13 +7,12 @@ */ import { - ANIMATION_QUEUE, AnimationCallbackEvent, AnimationFunction, MAX_ANIMATION_TIMEOUT, } from '../../animation/interfaces'; import {getLView, getCurrentTNode} from '../state'; -import {RENDERER, INJECTOR, CONTEXT, LView, ANIMATIONS} from '../interfaces/view'; +import {RENDERER, INJECTOR, CONTEXT, LView} from '../interfaces/view'; import {getNativeByTNode} from '../util/view_utils'; import {performanceMarkFeature} from '../../util/performance'; import {Renderer} from '../interfaces/renderer'; @@ -21,8 +20,6 @@ import {NgZone} from '../../zone'; import {determineLongestAnimation, allLeavingAnimations} from '../../animation/longest_animation'; import {TNode} from '../interfaces/node'; import {promiseWithResolvers} from '../../util/promise_with_resolvers'; -import {Injector} from '../../di'; -import {afterEveryRender} from '../after_render/hooks'; import { addAnimationToLView, @@ -47,6 +44,7 @@ import { trackEnterClasses, trackLeavingNodes, } from '../../animation/utils'; +import {initializeAnimationQueueScheduler, queueEnterAnimations} from '../../animation/queue'; /** * Instruction to handle the `animate.enter` behavior for class bindings. @@ -77,7 +75,11 @@ export function ɵɵanimateEnter(value: string | Function): typeof ɵɵanimateEn runEnterAnimation(lView, tNode, value), ); - queueEnterAnimations(lView); + initializeAnimationQueueScheduler(lView[INJECTOR]); + + // TODO(thePunderWoman): it's unclear why we need to queue animations here, but without this, + // animating through host bindings fails + queueEnterAnimations(lView[INJECTOR], getLViewEnterAnimations(lView)); return ɵɵanimateEnter; // For chaining } @@ -198,7 +200,11 @@ export function ɵɵanimateEnterListener(value: AnimationFunction): typeof ɵɵa runEnterAnimationFunction(lView, tNode, value), ); - queueEnterAnimations(lView); + initializeAnimationQueueScheduler(lView[INJECTOR]); + + // TODO(thePunderWoman): it's unclear why we need to queue animations here, but without this, + // animating through host bindings fails + queueEnterAnimations(lView[INJECTOR], getLViewEnterAnimations(lView)); return ɵɵanimateEnterListener; } @@ -244,7 +250,7 @@ export function ɵɵanimateLeave(value: string | Function): typeof ɵɵanimateLe runLeaveAnimations(lView, tNode, value), ); - enableAnimationQueueScheduler(lView[INJECTOR]); + initializeAnimationQueueScheduler(lView[INJECTOR]); return ɵɵanimateLeave; // For chaining } @@ -377,7 +383,7 @@ export function ɵɵanimateLeaveListener(value: AnimationFunction): typeof ɵɵa runLeaveAnimationFunction(lView, tNode, value), ); - enableAnimationQueueScheduler(lView[INJECTOR]); + initializeAnimationQueueScheduler(lView[INJECTOR]); return ɵɵanimateLeaveListener; // For chaining } @@ -465,38 +471,3 @@ function runLeaveAnimationFunction( // Ensure cleanup if the LView is destroyed before the animation runs. return {promise, resolve}; } - -function queueEnterAnimations(lView: LView) { - enableAnimationQueueScheduler(lView[INJECTOR]); - const enterAnimations = lView[ANIMATIONS]?.enter; - if (enterAnimations) { - const animationQueue = lView[INJECTOR].get(ANIMATION_QUEUE); - for (const [_, nodeAnimations] of enterAnimations) { - for (const animateFn of nodeAnimations.animateFns) { - animationQueue.queue.add(animateFn); - } - } - } -} - -function enableAnimationQueueScheduler(injector: Injector) { - const animationQueue = injector.get(ANIMATION_QUEUE); - // We only need to schedule the animation queue runner once per application. - if (!animationQueue.isScheduled) { - afterEveryRender( - () => { - runQueuedAnimations(injector); - }, - {injector}, - ); - animationQueue.isScheduled = true; - } -} - -function runQueuedAnimations(injector: Injector) { - const animationQueue = injector.get(ANIMATION_QUEUE); - for (let animateFn of animationQueue.queue) { - animateFn(); - } - animationQueue.queue.clear(); -} diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 923597f89774..255d04b03f0e 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -82,8 +82,8 @@ import {profiler} from './profiler'; import {ProfilerEvent} from './profiler_types'; import {getLViewParent, getNativeByTNode, unwrapRNode} from './util/view_utils'; import {allLeavingAnimations} from '../animation/longest_animation'; -import {ANIMATION_QUEUE} from '../animation/interfaces'; import {Injector} from '../di'; +import {addToAnimationQueue, queueEnterAnimations} from '../animation/queue'; const enum WalkTNodeTreeAction { /** node create in the native environment. Run on initial creation. */ @@ -110,10 +110,7 @@ function maybeQueueEnterAnimation( ): void { const enterAnimations = parentLView?.[ANIMATIONS]?.enter; if (parent !== null && enterAnimations && enterAnimations.has(tNode.index)) { - const animationQueue = injector.get(ANIMATION_QUEUE); - for (const animateFn of enterAnimations.get(tNode.index)!.animateFns) { - animationQueue.queue.add(animateFn); - } + queueEnterAnimations(injector, enterAnimations); } } @@ -182,17 +179,6 @@ function applyToElementOrContainer( } } -function addToAnimationQueue(injector: Injector, animationFns: Function | Function[]) { - const animationQueue = injector.get(ANIMATION_QUEUE); - if (Array.isArray(animationFns)) { - for (const animateFn of animationFns) { - animationQueue.queue.add(animateFn); - } - } else { - animationQueue.queue.add(animationFns); - } -} - /** * Removes all DOM elements associated with a view. * diff --git a/packages/core/test/acceptance/animation_spec.ts b/packages/core/test/acceptance/animation_spec.ts index 92a205bf566d..94044029c93a 100644 --- a/packages/core/test/acceptance/animation_spec.ts +++ b/packages/core/test/acceptance/animation_spec.ts @@ -9,15 +9,19 @@ import {NgFor} from '@angular/common'; import {ViewEncapsulation} from '@angular/compiler'; import { + AfterViewInit, AnimationCallbackEvent, + ChangeDetectionStrategy, Component, computed, Directive, ElementRef, NgModule, + OnDestroy, provideZonelessChangeDetection, signal, ViewChild, + ViewContainerRef, } from '@angular/core'; import {fakeAsync, TestBed, tick} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; @@ -25,6 +29,7 @@ import {isNode} from '@angular/private/testing'; import {tickAnimationFrames} from '../animation_utils/tick_animation_frames'; import {BrowserTestingModule, platformBrowserTesting} from '@angular/platform-browser/testing'; import {NoopAnimationsModule} from '@angular/platform-browser/animations'; +import {ComponentRef} from '@angular/core/src/render3'; @NgModule({ providers: [provideZonelessChangeDetection()], @@ -2022,4 +2027,121 @@ describe('Animation', () => { expect(fixture.debugElement.queryAll(By.css('p')).length).toBe(0); })); }); + + describe('animation queue timing', () => { + it('should run animations with a fresh componentRef after destroy', fakeAsync(() => { + const animateStyles = ` + .fade { + animation: fade-out 500ms; + } + @keyframes fade-out { + from { + opacity: 1; + } + to { + opacity: 0; + } + } + `; + + @Component({ + selector: 'app-control-panel', + template: ` + @if (step() === 0) { +

+ THIS SHOULD NOT BE HERE +

+ } + @if (step() === 1) { +

THIS SHOULD BE ALL THERE IS

+ } + `, + changeDetection: ChangeDetectionStrategy.OnPush, + }) + class StepperComponent { + readonly step = signal(0); + } + + @Component({ + selector: 'app-dynamic', + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, + }) + class DynamicComponent implements AfterViewInit, OnDestroy { + @ViewChild('dynamicComponent', {read: ViewContainerRef}) + dynamicComponent!: ViewContainerRef; + + constructor() { + this.componentRef = null; + } + + protected componentRef: ComponentRef | null; + + ngAfterViewInit(): void { + this.componentRef = this.dynamicComponent.createComponent( + StepperComponent, + ) as ComponentRef; + this.componentRef!.changeDetectorRef.detectChanges(); + + this.componentRef!.instance.step.set(1); + } + + ngOnDestroy(): void { + this.componentRef?.destroy(); + } + } + + @Component({ + selector: 'test-cmp', + imports: [DynamicComponent], + template: ` +
+ @if (show()) { + + } +
+ `, + encapsulation: ViewEncapsulation.None, + }) + class TestComponent { + show = signal(true); + + toggleOverlay() { + this.show.update((show) => !show); + } + } + TestBed.configureTestingModule({animationsEnabled: true}); + + const fixture = TestBed.createComponent(TestComponent); + const cmp = fixture.componentInstance; + fixture.detectChanges(); + + expect(fixture.debugElement.query(By.css('p.all-there-is'))).not.toBeNull(); + expect(fixture.debugElement.query(By.css('p.not-here.fade-out'))).not.toBeNull(); + + // Finish the leave animation to ensure it is removed + tickAnimationFrames(1); + + // verify element is removed post animation + expect(fixture.debugElement.query(By.css('p.not-here'))).toBeNull(); + + cmp.toggleOverlay(); + fixture.detectChanges(); + + // show is false. Nothing should be present. + expect(fixture.debugElement.query(By.css('p.all-there-is'))).toBeNull(); + expect(fixture.debugElement.query(By.css('p.not-here'))).toBeNull(); + + cmp.toggleOverlay(); + fixture.detectChanges(); + + expect(fixture.debugElement.query(By.css('p.not-here'))).not.toBeNull(); + + tickAnimationFrames(1); + + // show is true. Only one element should be present. + expect(fixture.debugElement.query(By.css('p.all-there-is'))).not.toBeNull(); + expect(fixture.debugElement.query(By.css('p.not-here'))).toBeNull(); + })); + }); }); diff --git a/packages/core/test/acceptance/view_container_ref_spec.ts b/packages/core/test/acceptance/view_container_ref_spec.ts index 47f02ef9fd40..9e9f093a0778 100644 --- a/packages/core/test/acceptance/view_container_ref_spec.ts +++ b/packages/core/test/acceptance/view_container_ref_spec.ts @@ -48,7 +48,7 @@ import {ComponentFixture, TestBed, TestComponentRenderer} from '../../testing'; import {clearTranslations, loadTranslations} from '@angular/localize'; import {By, DomSanitizer} from '@angular/platform-browser'; import {expect} from '@angular/private/testing/matchers'; -import {ANIMATION_QUEUE} from '../../src/animation/interfaces'; +import {ANIMATION_QUEUE} from '../../src/animation/queue'; describe('ViewContainerRef', () => { /** diff --git a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json index be2b1c80a5a7..e2dc986023b1 100644 --- a/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json +++ b/packages/core/test/bundling/animations-standalone/bundle.golden_symbols.json @@ -748,6 +748,7 @@ "providerToFactory", "providerToRecord", "publishSignalConfiguration", + "queueEnterAnimations", "refreshContentQueries", "refreshView", "registerFailed", diff --git a/packages/core/test/bundling/defer/bundle.golden_symbols.json b/packages/core/test/bundling/defer/bundle.golden_symbols.json index 13def30e155a..9e3241081579 100644 --- a/packages/core/test/bundling/defer/bundle.golden_symbols.json +++ b/packages/core/test/bundling/defer/bundle.golden_symbols.json @@ -650,6 +650,7 @@ "providerToFactory", "providerToRecord", "publishSignalConfiguration", + "queueEnterAnimations", "refreshContentQueries", "refreshView", "registerHostBindingOpCodes", diff --git a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json index b778dc651011..2ac115226db7 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -865,6 +865,7 @@ "providerToRecord", "providersResolver", "publishSignalConfiguration", + "queueEnterAnimations", "readableStreamLikeToAsyncGenerator", "refreshContentQueries", "refreshView", diff --git a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json index de1f6bc9a09d..c53db21387b0 100644 --- a/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_template_driven/bundle.golden_symbols.json @@ -863,6 +863,7 @@ "providerToRecord", "providersResolver", "publishSignalConfiguration", + "queueEnterAnimations", "readableStreamLikeToAsyncGenerator", "refreshContentQueries", "refreshView", diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 824f5eef5cf9..9421f0bcc961 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -686,6 +686,7 @@ "providerToFactory", "providerToRecord", "publishSignalConfiguration", + "queueEnterAnimations", "readableStreamLikeToAsyncGenerator", "refreshContentQueries", "refreshView", diff --git a/packages/core/test/bundling/router/bundle.golden_symbols.json b/packages/core/test/bundling/router/bundle.golden_symbols.json index 06e0904670d1..da799467a661 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -974,6 +974,7 @@ "providerToFactory", "providerToRecord", "publishSignalConfiguration", + "queueEnterAnimations", "readableStreamLikeToAsyncGenerator", "recognize", "recognize", diff --git a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json index 13e175c0dd1f..b71e11194ec6 100644 --- a/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json +++ b/packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json @@ -545,6 +545,7 @@ "providerToFactory", "providerToRecord", "publishSignalConfiguration", + "queueEnterAnimations", "refreshContentQueries", "refreshView", "registerHostBindingOpCodes",