Revert "refactor(core): Ensure DOM removal happens when no app views …

…need refresh (angular#55132)"

This reverts commit a07ea06.

packages/animations/browser/src/create_engine.ts

@@ -6,17 +6,33 @@
  * 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): AnimationEngine {
+export function createEngine(
+  type: 'animations' | 'noop',
+  doc: Document,
+  scheduler: ChangeDetectionScheduler | null,
+): AnimationEngine {
   // TODO: find a way to make this tree shakable.
   if (type === 'noop') {
-    return new AnimationEngine(doc, new NoopAnimationDriver(), new NoopAnimationStyleNormalizer());
+    return new AnimationEngine(
+      doc,
+      new NoopAnimationDriver(),
+      new NoopAnimationStyleNormalizer(),
+      scheduler,
+    );
   }
 
-  return new AnimationEngine(doc, new WebAnimationsDriver(), new WebAnimationsStyleNormalizer());
+  return new AnimationEngine(
+    doc,
+    new WebAnimationsDriver(),
+    new WebAnimationsStyleNormalizer(),
+    scheduler,
+  );
 }

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

@@ -6,6 +6,7 @@
  * 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';
@@ -32,8 +33,14 @@ export class AnimationEngine {
     doc: Document,
     private _driver: AnimationDriver,
     private _normalizer: AnimationStyleNormalizer,
+    scheduler: ChangeDetectionScheduler | null,
   ) {
-    this._transitionEngine = new TransitionAnimationEngine(doc.body, _driver, _normalizer);
+    this._transitionEngine = new TransitionAnimationEngine(
+      doc.body,
+      _driver,
+      _normalizer,
+      scheduler,
+    );
     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,7 +14,10 @@ import {
   ɵPRE_STYLE as PRE_STYLE,
   ɵStyleDataMap,
 } from '@angular/animations';
-import {ɵWritable as Writable} from '@angular/core';
+import {
+  ɵChangeDetectionScheduler as ChangeDetectionScheduler,
+  ɵWritable as Writable,
+} from '@angular/core';
 
 import {AnimationTimelineInstruction} from '../dsl/animation_timeline_instruction';
 import {AnimationTransitionFactory} from '../dsl/animation_transition_factory';
@@ -49,7 +52,6 @@ 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';
@@ -622,6 +624,7 @@ export class TransitionAnimationEngine {
     public bodyNode: any,
     public driver: AnimationDriver,
     private _normalizer: AnimationStyleNormalizer,
+    private readonly scheduler: ChangeDetectionScheduler | null,
   ) {}
 
   get queuedPlayers(): TransitionAnimationPlayer[] {
@@ -813,6 +816,7 @@ 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,7 +27,6 @@ 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';
@@ -163,10 +162,6 @@ 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 {
@@ -551,11 +546,6 @@ 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) {
@@ -577,11 +567,6 @@ 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
@@ -686,6 +671,10 @@ 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,7 +6,6 @@
  * 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';
@@ -37,7 +36,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(NotificationType.RefreshViews);
+  lView[ENVIRONMENT].changeDetectionScheduler?.notify();
   while (lView) {
     lView[FLAGS] |= dirtyBitsToUse;
     const parent = getLViewParent(lView);

packages/core/src/render3/node_manipulation.ts

@@ -174,7 +174,9 @@ 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.
+  // 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).
   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(NotificationType.RefreshViews);
+  lView[ENVIRONMENT].changeDetectionScheduler?.notify();
   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,22 +83,20 @@ describe('renderer factory lifecycle', () => {
 
   it('should work with a component', () => {
     const fixture = TestBed.createComponent(SomeComponent);
-    fixture.componentRef.changeDetectorRef.detectChanges();
-    expect(logs).toEqual([
-      'create', 'create', 'begin', 'end', 'begin', 'some_component create', 'some_component update',
-      'end'
-    ]);
+    fixture.detectChanges();
+    expect(logs).toEqual(
+        ['create', 'create', 'begin', 'some_component create', 'some_component update', 'end']);
     logs = [];
-    fixture.componentRef.changeDetectorRef.detectChanges();
+    fixture.detectChanges();
     expect(logs).toEqual(['begin', 'some_component update', 'end']);
   });
 
   it('should work with a component which throws', () => {
     expect(() => {
       const fixture = TestBed.createComponent(SomeComponentWhichThrows);
-      fixture.componentRef.changeDetectorRef.detectChanges();
+      fixture.detectChanges();
     }).toThrow();
-    expect(logs).toEqual(['create', 'create', 'begin', 'end', 'begin', 'end']);
+    expect(logs).toEqual(['create', 'create', 'begin', 'end']);
   });
 
   it('should pass in the component styles directly into the underlying renderer', () => {
@@ -341,7 +339,7 @@ function getAnimationRendererFactory2(document: Document): RendererFactory2 {
   return new ɵAnimationRendererFactory(
       getRendererFactory2(document),
       new ɵAnimationEngine(
-          document, new MockAnimationDriver(), new ɵNoopAnimationStyleNormalizer()),
+          document, new MockAnimationDriver(), new ɵNoopAnimationStyleNormalizer(), null),
       fakeNgZone);
 }
 

packages/core/test/change_detection_scheduler_spec.ts

@@ -333,6 +333,10 @@ 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,7 +44,11 @@ export class AsyncAnimationRendererFactory implements OnDestroy, RendererFactory
     private zone: NgZone,
     private animationType: 'animations' | 'noop',
     private moduleImpl?: Promise<{
-      ɵcreateEngine: (type: 'animations' | 'noop', doc: Document) => AnimationEngine;
+      ɵcreateEngine: (
+        type: 'animations' | 'noop',
+        doc: Document,
+        scheduler: ChangeDetectionScheduler | null,
+      ) => AnimationEngine;
       ɵAnimationRendererFactory: typeof AnimationRendererFactory;
     }>,
   ) {}
@@ -79,7 +83,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._engine = ɵcreateEngine(this.animationType, this.doc, this.scheduler);
         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);
+    super(doc, driver, normalizer, inject(ChangeDetectionScheduler, {optional: true}));
   }
 
   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.changeDetectorRef.detectChanges();
+      fixture.detectChanges();
       expect(renderer.log).toEqual(['begin', 'end']);
 
       renderer.log = [];
-      fixture.changeDetectorRef.detectChanges();
+      fixture.detectChanges();
       expect(renderer.log).toEqual(['begin', 'end']);
     });
   });