From 911d6822cb18dabf4f72312dfc2e2ef9904bf6c2 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 (#64441) 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 PR Close #64441 --- 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 52d40a32296..a8b4fbb0489 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 00000000000..4bc53cf1206 --- /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 db70efa4946..916f60493fe 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 923597f8977..255d04b03f0 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 92a205bf566..94044029c93 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 47f02ef9fd4..9e9f093a077 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 d9e92fce60d..ab8628db628 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 5318b1bde13..da834e72bd9 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 16fd2fc87df..3995800407d 100644 --- a/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json +++ b/packages/core/test/bundling/forms_reactive/bundle.golden_symbols.json @@ -870,6 +870,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 a84a6170522..d04f68c0d7a 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 @@ -869,6 +869,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 735abe70db4..ec2111173c6 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -688,6 +688,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 5b2965634ad..73422b015a5 100644 --- a/packages/core/test/bundling/router/bundle.golden_symbols.json +++ b/packages/core/test/bundling/router/bundle.golden_symbols.json @@ -973,6 +973,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 6ab2aa8f7b3..23078924315 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",