refactor(core): Ensure DOM removal happens when no app views need ref…

…resh

This change ensures that `ApplicationRef.tick` flushes animations by
calling `rendererFactory2.end`. This might not have happened before if
there were no views that needed to be refreshed.

This is also likely to fix a potential regression caused by angular#53718 even
in zone apps where animations don't get flushed when no views attached
to ApplicationRef are dirty.

packages/animations/browser/src/create_engine.ts

@@ -6,33 +6,17 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ɵChangeDetectionScheduler as ChangeDetectionScheduler} from '@angular/core';
-
 import {NoopAnimationStyleNormalizer} from './dsl/style_normalization/animation_style_normalizer';
 import {WebAnimationsStyleNormalizer} from './dsl/style_normalization/web_animations_style_normalizer';
 import {NoopAnimationDriver} from './render/animation_driver';
 import {AnimationEngine} from './render/animation_engine_next';
 import {WebAnimationsDriver} from './render/web_animations/web_animations_driver';
 
-export function createEngine(
-  type: 'animations' | 'noop',
-  doc: Document,
-  scheduler: ChangeDetectionScheduler | null,
-): AnimationEngine {
+export function createEngine(type: 'animations' | 'noop', doc: Document): AnimationEngine {
   // TODO: find a way to make this tree shakable.
   if (type === 'noop') {
-    return new AnimationEngine(
-      doc,
-      new NoopAnimationDriver(),
-      new NoopAnimationStyleNormalizer(),
-      scheduler,
-    );
+    return new AnimationEngine(doc, new NoopAnimationDriver(), new NoopAnimationStyleNormalizer());
   }
 
-  return new AnimationEngine(
-    doc,
-    new WebAnimationsDriver(),
-    new WebAnimationsStyleNormalizer(),
-    scheduler,
-  );
+  return new AnimationEngine(doc, new WebAnimationsDriver(), new WebAnimationsStyleNormalizer());
 }

packages/animations/browser/src/render/animation_engine_next.ts

@@ -6,7 +6,6 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {AnimationMetadata, AnimationPlayer, AnimationTriggerMetadata} from '@angular/animations';
-import {ɵChangeDetectionScheduler as ChangeDetectionScheduler} from '@angular/core';
 
 import {TriggerAst} from '../dsl/animation_ast';
 import {buildAnimationAst} from '../dsl/animation_ast_builder';
@@ -33,14 +32,8 @@ export class AnimationEngine {
     doc: Document,
     private _driver: AnimationDriver,
     private _normalizer: AnimationStyleNormalizer,
-    scheduler: ChangeDetectionScheduler | null,
   ) {
-    this._transitionEngine = new TransitionAnimationEngine(
-      doc.body,
-      _driver,
-      _normalizer,
-      scheduler,
-    );
+    this._transitionEngine = new TransitionAnimationEngine(doc.body, _driver, _normalizer);
     this._timelineEngine = new TimelineAnimationEngine(doc.body, _driver, _normalizer);
 
     this._transitionEngine.onRemovalComplete = (element: any, context: any) =>

packages/animations/browser/src/render/transition_animation_engine.ts

@@ -14,10 +14,7 @@ import {
   ɵPRE_STYLE as PRE_STYLE,
   ɵStyleDataMap,
 } from '@angular/animations';
-import {
-  ɵChangeDetectionScheduler as ChangeDetectionScheduler,
-  ɵWritable as Writable,
-} from '@angular/core';
+import {ɵWritable as Writable} from '@angular/core';
 
 import {AnimationTimelineInstruction} from '../dsl/animation_timeline_instruction';
 import {AnimationTransitionFactory} from '../dsl/animation_transition_factory';
@@ -52,6 +49,7 @@ import {
   normalizeKeyframes,
   optimizeGroupPlayer,
 } from './shared';
+import {NotificationType} from '@angular/core/src/change_detection/scheduling/zoneless_scheduling';
 
 const QUEUED_CLASSNAME = 'ng-animate-queued';
 const QUEUED_SELECTOR = '.ng-animate-queued';
@@ -624,7 +622,6 @@ export class TransitionAnimationEngine {
     public bodyNode: any,
     public driver: AnimationDriver,
     private _normalizer: AnimationStyleNormalizer,
-    private readonly scheduler: ChangeDetectionScheduler | null,
   ) {}
 
   get queuedPlayers(): TransitionAnimationPlayer[] {
@@ -816,7 +813,6 @@ export class TransitionAnimationEngine {
 
   removeNode(namespaceId: string, element: any, context: any): void {
     if (isElementNode(element)) {
-      this.scheduler?.notify();
       const ns = namespaceId ? this._fetchNamespace(namespaceId) : null;
       if (ns) {
         ns.removeNode(element, context);

packages/core/src/application/application_ref.ts

@@ -27,6 +27,7 @@ import {ComponentFactoryResolver} from '../linker/component_factory_resolver';
 import {NgModuleRef} from '../linker/ng_module_factory';
 import {ViewRef} from '../linker/view_ref';
 import {PendingTasks} from '../pending_tasks';
+import {RendererFactory2} from '../render/api';
 import {AfterRenderEventManager} from '../render3/after_render_hooks';
 import {ComponentFactory as R3ComponentFactory} from '../render3/component_ref';
 import {isStandalone} from '../render3/definition';
@@ -162,6 +163,10 @@ export interface BootstrapOptions {
 /** Maximum number of times ApplicationRef will refresh all attached views in a single tick. */
 const MAXIMUM_REFRESH_RERUNS = 10;
 
+// This is a temporary type to represent an instance of an R3Injector, which can be destroyed.
+// The type will be replaced with a different one once destroyable injector type is available.
+type DestroyableInjector = Injector&{destroy?: Function, destroyed?: boolean};
+
 export function _callAndReportToErrorHandler(
     errorHandler: ErrorHandler, ngZone: NgZone, callback: () => any): any {
   try {
@@ -546,6 +551,11 @@ export class ApplicationRef {
   }
 
   private detectChangesInAttachedViews(refreshViews: boolean) {
+    let rendererFactory: RendererFactory2|null = null;
+    if (!(this._injector as DestroyableInjector).destroyed) {
+      rendererFactory = this._injector.get(RendererFactory2, null, {optional: true});
+    }
+
     let runs = 0;
     const afterRenderEffectManager = this.afterRenderEffectManager;
     while (runs < MAXIMUM_REFRESH_RERUNS) {
@@ -567,6 +577,11 @@ export class ApplicationRef {
         continue;
       }
 
+      // Flush animations before running afterRender hooks
+      // This might not have happened if no views were refreshed above
+      rendererFactory?.begin?.();
+      rendererFactory?.end?.();
+
       afterRenderEffectManager.execute();
       // If after running all afterRender callbacks we have no more views that need to be refreshed,
       // we can break out of the loop
@@ -671,10 +686,6 @@ export class ApplicationRef {
           ngDevMode && 'This instance of the `ApplicationRef` has already been destroyed.');
     }
 
-    // This is a temporary type to represent an instance of an R3Injector, which can be destroyed.
-    // The type will be replaced with a different one once destroyable injector type is available.
-    type DestroyableInjector = Injector&{destroy?: Function, destroyed?: boolean};
-
     const injector = this._injector as DestroyableInjector;
 
     // Check that this injector instance supports destroy operation.

packages/core/src/render3/instructions/mark_view_dirty.ts

@@ -6,6 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {NotificationType} from '../../change_detection/scheduling/zoneless_scheduling';
 import {isRootView} from '../interfaces/type_checks';
 import {ENVIRONMENT, FLAGS, LView, LViewFlags} from '../interfaces/view';
 import {isRefreshingViews} from '../state';
@@ -36,7 +37,7 @@ export function markViewDirty(lView: LView): LView|null {
       // afterRender hooks as well as animation listeners which execute after detecting
       // changes in a view when the render factory flushes.
       LViewFlags.RefreshView | LViewFlags.Dirty;
-  lView[ENVIRONMENT].changeDetectionScheduler?.notify();
+  lView[ENVIRONMENT].changeDetectionScheduler?.notify(NotificationType.RefreshViews);
   while (lView) {
     lView[FLAGS] |= dirtyBitsToUse;
     const parent = getLViewParent(lView);

packages/core/src/render3/node_manipulation.ts

@@ -174,9 +174,7 @@ export function addViewToDOM(
  * @param lView the `LView` to be detached.
  */
 export function detachViewFromDOM(tView: TView, lView: LView) {
-  // When we remove a view from the DOM, we need to rerun afterRender hooks
-  // We don't necessarily needs to run change detection. DOM removal only requires
-  // change detection if animations are enabled (this notification is handled by animations).
+  // When we remove a view from the DOM, we need to rerun afterRender hooks.
   lView[ENVIRONMENT].changeDetectionScheduler?.notify(NotificationType.AfterRenderHooks);
   applyView(tView, lView, lView[RENDERER], WalkTNodeTreeAction.Detach, null, null);
 }

packages/core/src/render3/util/view_utils.ts

@@ -225,7 +225,7 @@ export function updateAncestorTraversalFlagsOnAttach(lView: LView) {
  * flag is already `true` or the `lView` is detached.
  */
 export function markAncestorsForTraversal(lView: LView) {
-  lView[ENVIRONMENT].changeDetectionScheduler?.notify();
+  lView[ENVIRONMENT].changeDetectionScheduler?.notify(NotificationType.RefreshViews);
   let parent = getLViewParent(lView);
   while (parent !== null) {
     // We stop adding markers to the ancestors once we reach one that already has the marker. This

packages/core/test/acceptance/renderer_factory_spec.ts

@@ -83,20 +83,22 @@ describe('renderer factory lifecycle', () => {
 
   it('should work with a component', () => {
     const fixture = TestBed.createComponent(SomeComponent);
-    fixture.detectChanges();
-    expect(logs).toEqual(
-        ['create', 'create', 'begin', 'some_component create', 'some_component update', 'end']);
+    fixture.componentRef.changeDetectorRef.detectChanges();
+    expect(logs).toEqual([
+      'create', 'create', 'begin', 'end', 'begin', 'some_component create', 'some_component update',
+      'end'
+    ]);
     logs = [];
-    fixture.detectChanges();
+    fixture.componentRef.changeDetectorRef.detectChanges();
     expect(logs).toEqual(['begin', 'some_component update', 'end']);
   });
 
   it('should work with a component which throws', () => {
     expect(() => {
       const fixture = TestBed.createComponent(SomeComponentWhichThrows);
-      fixture.detectChanges();
+      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', () => {
@@ -339,7 +341,7 @@ function getAnimationRendererFactory2(document: Document): RendererFactory2 {
   return new ɵAnimationRendererFactory(
       getRendererFactory2(document),
       new ɵAnimationEngine(
-          document, new MockAnimationDriver(), new ɵNoopAnimationStyleNormalizer(), null),
+          document, new MockAnimationDriver(), new ɵNoopAnimationStyleNormalizer()),
       fakeNgZone);
 }
 

packages/core/test/change_detection_scheduler_spec.ts

@@ -333,10 +333,6 @@ describe('Angular with zoneless enabled', () => {
       await whenStable();
       expect(host.innerHTML).toEqual('<dynamic-cmp>binding</dynamic-cmp>');
 
-      const component2 = createComponent(DynamicCmp, {environmentInjector});
-      // TODO(atscott): Only needed because renderFactory will not run if ApplicationRef has no
-      // views This should likely be fixed in ApplicationRef
-      appRef.attachView(component2.hostView);
       appRef.detachView(component.hostView);
       // DOM is not synchronously removed because change detection hasn't run
       expect(host.innerHTML).toEqual('<dynamic-cmp>binding</dynamic-cmp>');

packages/platform-browser/animations/async/src/async_animation_renderer.ts

@@ -44,11 +44,7 @@ export class AsyncAnimationRendererFactory implements OnDestroy, RendererFactory
     private zone: NgZone,
     private animationType: 'animations' | 'noop',
     private moduleImpl?: Promise<{
-      ɵcreateEngine: (
-        type: 'animations' | 'noop',
-        doc: Document,
-        scheduler: ChangeDetectionScheduler | null,
-      ) => AnimationEngine;
+      ɵcreateEngine: (type: 'animations' | 'noop', doc: Document) => AnimationEngine;
       ɵAnimationRendererFactory: typeof AnimationRendererFactory;
     }>,
   ) {}
@@ -83,7 +79,7 @@ export class AsyncAnimationRendererFactory implements OnDestroy, RendererFactory
       .then(({ɵcreateEngine, ɵAnimationRendererFactory}) => {
         // We can't create the renderer yet because we might need the hostElement and the type
         // Both are provided in createRenderer().
-        this._engine = ɵcreateEngine(this.animationType, this.doc, this.scheduler);
+        this._engine = ɵcreateEngine(this.animationType, this.doc);
         const rendererFactory = new ɵAnimationRendererFactory(
           this.delegate,
           this._engine,

packages/platform-browser/animations/src/providers.ts

@@ -39,7 +39,7 @@ export class InjectableAnimationEngine extends AnimationEngine implements OnDest
     driver: AnimationDriver,
     normalizer: AnimationStyleNormalizer,
   ) {
-    super(doc, driver, normalizer, inject(ChangeDetectionScheduler, {optional: true}));
+    super(doc, driver, normalizer);
   }
 
   ngOnDestroy(): void {

packages/platform-browser/animations/test/animation_renderer_spec.ts

@@ -351,11 +351,11 @@ import {el} from '../../testing/src/browser_util';
       const cmp = fixture.componentInstance;
 
       renderer.log = [];
-      fixture.detectChanges();
+      fixture.changeDetectorRef.detectChanges();
       expect(renderer.log).toEqual(['begin', 'end']);
 
       renderer.log = [];
-      fixture.detectChanges();
+      fixture.changeDetectorRef.detectChanges();
       expect(renderer.log).toEqual(['begin', 'end']);
     });
   });