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

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 angular#58075

PR Close angular#58089

Author: JeanMeche

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);
-}

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,

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', () => {

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()}}'})

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
       ]);
     }),
   );