fix(core): Flush animations when no component has been checked (#58089)

JeanMeche · atscott · commit 0b69b6192962 · 2025-04-09T11:12:54.000-07:00
When Angular runs application synchronization automatically, animations are now guaranteed to be flushed, regardless of whether change detection was run on any components attached to `ApplicationRef`. This most frequently affects animations related to component removal where the DOM element for the component would previously not be removed due to animations not being flushed BREAKING CHANGE: Animations are guaranteed to be flushed when Angular runs automatic change detection or manual calls to `ApplicationRef.tick`. Prior to this change, animations would not be flushed in some situations if change detection did not run on any views attached to the application. This change can affect tests which may rely on the old behavior, often by making assertions on DOM elements that should have been removed but weren't because DOM removal is delayed until animations are flushed. fixes #58075 PR Close #58089
diff --git a/packages/core/src/application/application_ref.ts b/packages/core/src/application/application_ref.ts
@@ -666,6 +666,7 @@ export class ApplicationRef {
     }
 
     // First check dirty views, if there are any.
+    let ranDetectChanges = false;
     if (this.dirtyFlags & ApplicationRefDirtyFlags.ViewTreeAny) {
       // Change detection on views starts in targeted mode (only check components if they're
       // marked as dirty) unless global checking is specifically requested via APIs like
@@ -680,7 +681,21 @@ export class ApplicationRef {
 
       // Check all potentially dirty views.
       for (let {_lView} of this.allViews) {
-        detectChangesInViewIfRequired(_lView, useGlobalCheck, this.zonelessEnabled);
+        // When re-checking, only check views which actually need it.
+        if (!useGlobalCheck && !requiresRefreshOrTraversal(_lView)) {
+          continue;
+        }
+
+        const mode =
+          useGlobalCheck && !this.zonelessEnabled
+            ? // Global mode includes `CheckAlways` views.
+              // When using zoneless, all root views must be explicitly marked for refresh, even if they are
+              // `CheckAlways`.
+              ChangeDetectionMode.Global
+            : // Only refresh views with the `RefreshView` flag or views is a changed signal
+              ChangeDetectionMode.Targeted;
+        detectChangesInternal(_lView, mode);
+        ranDetectChanges = true;
       }
 
       // If `markForCheck()` was called during view checking, it will have set the `ViewTreeCheck`
@@ -698,7 +713,8 @@ export class ApplicationRef {
         // hooks.
         return;
       }
-    } else {
+    }
+    if (!ranDetectChanges) {
       // If we skipped refreshing views above, there might still be unflushed animations
       // because we never called `detectChangesInternal` on the views.
       this._rendererFactory?.begin?.();
@@ -899,24 +915,3 @@ export const enum ApplicationRefDirtyFlags {
    */
   RootEffects = 0b00010000,
 }
-
-export function detectChangesInViewIfRequired(
-  lView: LView,
-  isFirstPass: boolean,
-  zonelessEnabled: boolean,
-) {
-  // When re-checking, only check views which actually need it.
-  if (!isFirstPass && !requiresRefreshOrTraversal(lView)) {
-    return;
-  }
-
-  const mode =
-    isFirstPass && !zonelessEnabled
-      ? // The first pass is always in Global mode, which includes `CheckAlways` views.
-        // When using zoneless, all root views must be explicitly marked for refresh, even if they are
-        // `CheckAlways`.
-        ChangeDetectionMode.Global
-      : // Only refresh views with the `RefreshView` flag or views is a changed signal
-        ChangeDetectionMode.Targeted;
-  detectChangesInternal(lView, mode);
-}
diff --git a/packages/core/src/core_private_export.ts b/packages/core/src/core_private_export.ts
@@ -22,7 +22,6 @@ export {
   type NavigationDestination as ɵNavigationDestination,
 } from '../primitives/dom-navigation';
 export {setAlternateWeakRefImpl as ɵsetAlternateWeakRefImpl} from '../primitives/signals';
-export {detectChangesInViewIfRequired as ɵdetectChangesInViewIfRequired} from './application/application_ref';
 export {INTERNAL_APPLICATION_ERROR_HANDLER as ɵINTERNAL_APPLICATION_ERROR_HANDLER} from './error_handler';
 export {
   IMAGE_CONFIG as ɵIMAGE_CONFIG,
diff --git a/packages/core/test/acceptance/renderer_factory_spec.ts b/packages/core/test/acceptance/renderer_factory_spec.ts
@@ -114,6 +114,8 @@ describe('renderer factory lifecycle', () => {
       'create',
       'create',
       'begin',
+      'end',
+      'begin',
       'some_component create',
       'some_component update',
       'end',
@@ -128,7 +130,7 @@ describe('renderer factory lifecycle', () => {
       const fixture = TestBed.createComponent(SomeComponentWhichThrows);
       fixture.componentRef.changeDetectorRef.detectChanges();
     }).toThrow();
-    expect(logs).toEqual(['create', 'create', 'begin', 'end']);
+    expect(logs).toEqual(['create', 'create', 'begin', 'end', 'begin', 'end']);
   });
 
   it('should pass in the component styles directly into the underlying renderer', () => {
diff --git a/packages/core/test/change_detection_scheduler_spec.ts b/packages/core/test/change_detection_scheduler_spec.ts
@@ -14,6 +14,7 @@ import {
   ApplicationRef,
   ChangeDetectorRef,
   Component,
+  ComponentRef,
   createComponent,
   destroyPlatform,
   ElementRef,
@@ -51,6 +52,7 @@ import {RuntimeError, RuntimeErrorCode} from '../src/errors';
 import {scheduleCallbackWithRafRace} from '../src/util/callback_scheduler';
 import {ChangeDetectionSchedulerImpl} from '../src/change_detection/scheduling/zoneless_scheduling_impl';
 import {global} from '../src/util/global';
+import {provideNoopAnimations} from '@angular/platform-browser/animations';
 
 function isStable(injector = TestBed.inject(EnvironmentInjector)): boolean {
   return toSignal(injector.get(ApplicationRef).isStable, {requireSync: true, injector})();
@@ -375,6 +377,77 @@ describe('Angular with zoneless enabled', () => {
       expect(isStable()).toBe(false);
     });
 
+    it(
+      'when attaching view to ApplicationRef with animations',
+      withBody('<app></app>', async () => {
+        destroyPlatform();
+
+        @Component({
+          standalone: true,
+          template: `<p>Component created</p>`,
+        })
+        class DynamicComponent {
+          cdr = inject(ChangeDetectorRef);
+        }
+
+        @Component({
+          selector: 'app',
+          standalone: true,
+          template: `<main #outlet></main>`,
+        })
+        class App {
+          @ViewChild('outlet') outlet!: ElementRef<HTMLElement>;
+
+          envInjector = inject(EnvironmentInjector);
+          appRef = inject(ApplicationRef);
+          elementRef = inject(ElementRef);
+
+          createComponent() {
+            const host = document.createElement('div');
+            this.outlet.nativeElement.appendChild(host);
+
+            const ref = createComponent(DynamicComponent, {
+              environmentInjector: this.envInjector,
+              hostElement: host,
+            });
+
+            this.appRef.attachView(ref.hostView);
+
+            return ref;
+          }
+        }
+
+        const applicationRef = await bootstrapApplication(App, {
+          providers: [
+            provideZonelessChangeDetection(),
+            provideNoopAnimations(),
+            {provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID},
+          ],
+        });
+
+        const component = applicationRef.components[0] as ComponentRef<App>;
+        const appNativeElement = component.instance.elementRef.nativeElement;
+
+        await applicationRef.whenStable();
+        expect(appNativeElement.innerHTML).toEqual('<main></main>');
+
+        const ref: ComponentRef<DynamicComponent> = component.instance.createComponent();
+        await applicationRef.whenStable();
+        expect(appNativeElement.innerHTML).toContain('<p>Component created</p>');
+
+        // Similating a case where invoking destroy also schedules a CD.
+        ref.instance.cdr.markForCheck();
+        ref.destroy();
+
+        // DOM is not synchronously removed because change detection hasn't run
+        expect(appNativeElement.innerHTML).toContain('<p>Component created</p>');
+        await applicationRef.whenStable();
+
+        expect(isStable()).toBe(true);
+        expect(appNativeElement.innerHTML).toEqual('<main></main>');
+      }),
+    );
+
     it('when a stable subscription synchronously causes another notification', async () => {
       const val = signal('initial');
       @Component({template: '{{val()}}'})
diff --git a/packages/core/test/render3/change_detection_spec.ts b/packages/core/test/render3/change_detection_spec.ts
@@ -32,15 +32,15 @@ describe('change detection', () => {
       rendererFactory.end = () => log.push('end');
 
       const fixture = TestBed.createComponent(MyComponent);
-      fixture.detectChanges();
+      log.length = 0;
+      fixture.changeDetectorRef.detectChanges();
 
       expect(fixture.nativeElement.innerHTML).toEqual('works');
 
       expect(log).toEqual([
         'begin',
         'detect changes', // regular change detection cycle
         'end',
-        'detect changes', // check no changes cycle
       ]);
     }),
   );
