refactor(core): node removal notifies scheduler only when animations …

…are enabled (#53857)

Node removal is immediate and does not require change detection to run
when animations are not provided. This refactor makes the animation
engine notify the scheduler rather than doing it on all node removals.

PR Close #53857

packages/animations/browser/src/create_engine.ts

@@ -6,17 +6,23 @@
  * 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';
@@ -30,8 +31,9 @@ export class AnimationEngine {
 
   constructor(
       doc: Document, private _driver: AnimationDriver,
-      private _normalizer: AnimationStyleNormalizer) {
-    this._transitionEngine = new TransitionAnimationEngine(doc.body, _driver, _normalizer);
+      private _normalizer: AnimationStyleNormalizer, scheduler: ChangeDetectionScheduler|null) {
+    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

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 import {AnimationOptions, AnimationPlayer, AUTO_STYLE, NoopAnimationPlayer, ɵAnimationGroupPlayer as AnimationGroupPlayer, ɵ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';
@@ -547,7 +547,8 @@ export class TransitionAnimationEngine {
 
   constructor(
       public bodyNode: any, public driver: AnimationDriver,
-      private _normalizer: AnimationStyleNormalizer) {}
+      private _normalizer: AnimationStyleNormalizer,
+      private readonly scheduler: ChangeDetectionScheduler|null) {}
 
   get queuedPlayers(): TransitionAnimationPlayer[] {
     const players: TransitionAnimationPlayer[] = [];
@@ -738,6 +739,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/animations/browser/test/render/transition_animation_engine_spec.ts

@@ -38,7 +38,7 @@ describe('TransitionAnimationEngine', () => {
 
   function makeEngine(normalizer?: AnimationStyleNormalizer) {
     const engine = new TransitionAnimationEngine(
-        getBodyNode(), driver, normalizer || new NoopAnimationStyleNormalizer());
+        getBodyNode(), driver, normalizer || new NoopAnimationStyleNormalizer(), null);
     engine.createNamespace(DEFAULT_NAMESPACE_ID, element);
     return engine;
   }

packages/core/src/render3/node_manipulation.ts

@@ -173,9 +173,6 @@ export function addViewToDOM(
  * @param lView the `LView` to be detached.
  */
 export function detachViewFromDOM(tView: TView, lView: LView) {
-  // The scheduler must be notified because the animation engine is what actually does the DOM
-  // removal and only runs at the end of change detection.
-  lView[ENVIRONMENT].changeDetectionScheduler?.notify();
   applyView(tView, lView, lView[RENDERER], WalkTNodeTreeAction.Detach, null, null);
 }
 

packages/core/test/acceptance/renderer_factory_spec.ts

@@ -339,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

@@ -7,21 +7,21 @@
  */
 
 import {AsyncPipe} from '@angular/common';
-import {ApplicationRef, ChangeDetectorRef, Component, ComponentRef, createComponent, DebugElement, ElementRef, EnvironmentInjector, ErrorHandler, getDebugNode, inject, Injectable, Input, NgZone, PLATFORM_ID, signal, TemplateRef, Type, ViewChild, ViewContainerRef, ɵprovideZonelessChangeDetection as provideZonelessChangeDetection} from '@angular/core';
+import {PLATFORM_BROWSER_ID} from '@angular/common/src/platform_id';
+import {ApplicationRef, ChangeDetectorRef, Component, ComponentRef, createComponent, DebugElement, destroyPlatform, ElementRef, EnvironmentInjector, ErrorHandler, getDebugNode, inject, Injectable, Input, NgZone, PLATFORM_ID, signal, TemplateRef, Type, ViewChild, ViewContainerRef, ɵprovideZonelessChangeDetection as provideZonelessChangeDetection} from '@angular/core';
 import {toSignal} from '@angular/core/rxjs-interop';
 import {TestBed} from '@angular/core/testing';
+import {bootstrapApplication} from '@angular/platform-browser';
+import {withBody} from '@angular/private/testing';
 import {BehaviorSubject, firstValueFrom} from 'rxjs';
 import {filter, take, tap} from 'rxjs/operators';
 
 describe('Angular with NoopNgZone', () => {
-  function whenStable(): Promise<boolean> {
-    return firstValueFrom(TestBed.inject(EnvironmentInjector)
-                              .get(ApplicationRef)
-                              .isStable.pipe(filter(stable => stable)));
+  function whenStable(applicationRef = TestBed.inject(ApplicationRef)): Promise<boolean> {
+    return firstValueFrom(applicationRef.isStable.pipe(filter(stable => stable)));
   }
 
-  function isStable(): boolean {
-    const injector = TestBed.inject(EnvironmentInjector);
+  function isStable(injector = TestBed.inject(EnvironmentInjector)): boolean {
     return toSignal(injector.get(ApplicationRef).isStable, {requireSync: true, injector})();
   }
 
@@ -216,7 +216,7 @@ describe('Angular with NoopNgZone', () => {
       expect(componentRef.location.nativeElement.innerText).toEqual('binding');
     });
 
-    it('when destroying a view', async () => {
+    it('when destroying a view (with animations)', async () => {
       @Component({
         template: '{{"binding"}}',
         standalone: true,
@@ -249,10 +249,48 @@ describe('Angular with NoopNgZone', () => {
       await whenStable();
       expect(fixture.location.nativeElement.innerText).toEqual('binding');
       component2.destroy();
+      expect(isStable()).toBe(false);
       await whenStable();
       expect(fixture.location.nativeElement.innerText).toEqual('');
     });
 
+    it('when destroying a view (*no* animations)', withBody('<app></app>', async () => {
+         destroyPlatform();
+         @Component({
+           template: '{{"binding"}}',
+           standalone: true,
+         })
+         class DynamicCmp {
+           elementRef = inject(ElementRef);
+         }
+         @Component({
+           selector: 'app',
+           template: '<ng-template #ref></ng-template>',
+           standalone: true,
+         })
+         class App {
+           @ViewChild('ref', {read: ViewContainerRef}) viewContainer!: ViewContainerRef;
+         }
+         const applicationRef = await bootstrapApplication(App, {
+           providers: [
+             provideZonelessChangeDetection(),
+             {provide: PLATFORM_ID, useValue: PLATFORM_BROWSER_ID},
+           ]
+         });
+         const appViewRef = (applicationRef as any)._views[0] as {context: App, rootNodes: any[]};
+         await whenStable(applicationRef);
+
+         const component2 =
+             createComponent(DynamicCmp, {environmentInjector: applicationRef.injector});
+         appViewRef.context.viewContainer.insert(component2.hostView);
+         expect(isStable(applicationRef.injector)).toBe(false);
+         await whenStable(applicationRef);
+         component2.destroy();
+         expect(isStable(applicationRef.injector)).toBe(true);
+         expect(appViewRef.rootNodes[0].innerText).toEqual('');
+         destroyPlatform();
+       }));
+
     it('when attaching view to ApplicationRef', async () => {
       @Component({
         selector: 'dynamic-cmp',

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

@@ -7,13 +7,14 @@
  */
 
 import {ɵAnimationEngine as AnimationEngine, ɵAnimationRenderer as AnimationRenderer, ɵAnimationRendererFactory as AnimationRendererFactory} from '@angular/animations/browser';
-import {NgZone, Renderer2, RendererFactory2, RendererStyleFlags2, RendererType2, ɵAnimationRendererType as AnimationRendererType, ɵRuntimeError as RuntimeError} from '@angular/core';
+import {inject, NgZone, Renderer2, RendererFactory2, RendererStyleFlags2, RendererType2, ɵAnimationRendererType as AnimationRendererType, ɵChangeDetectionScheduler as ChangeDetectionScheduler, ɵRuntimeError as RuntimeError} from '@angular/core';
 import {ɵRuntimeErrorCode as RuntimeErrorCode} from '@angular/platform-browser';
 
 const ANIMATION_PREFIX = '@';
 
 export class AsyncAnimationRendererFactory implements RendererFactory2 {
   private _rendererFactoryPromise: Promise<AnimationRendererFactory>|null = null;
+  private readonly scheduler = inject(ChangeDetectionScheduler, {optional: true});
 
   /**
    *
@@ -22,7 +23,9 @@ export class AsyncAnimationRendererFactory implements RendererFactory2 {
   constructor(
       private doc: Document, private delegate: RendererFactory2, 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
       }>) {}
 
@@ -44,7 +47,7 @@ export class AsyncAnimationRendererFactory implements RendererFactory2 {
         .then(({ɵcreateEngine, ɵAnimationRendererFactory}) => {
           // We can't create the renderer yet because we might need the hostElement and the type
           // Both are provided in createRenderer().
-          const engine = ɵcreateEngine(this.animationType, this.doc);
+          const engine = ɵcreateEngine(this.animationType, this.doc, this.scheduler);
           const rendererFactory = new ɵAnimationRendererFactory(this.delegate, engine, this.zone);
           this.delegate = rendererFactory;
           return rendererFactory;

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

@@ -8,7 +8,7 @@
 import {animate, AnimationPlayer, AnimationTriggerMetadata, style, transition, trigger} from '@angular/animations';
 import {ɵAnimationEngine as AnimationEngine, ɵAnimationRenderer as AnimationRenderer, ɵAnimationRendererFactory as AnimationRendererFactory, ɵBaseAnimationRenderer as BaseAnimationRenderer} from '@angular/animations/browser';
 import {DOCUMENT} from '@angular/common';
-import {ANIMATION_MODULE_TYPE, Component, Injectable, NgZone, Renderer2, RendererFactory2, RendererType2, ViewChild} from '@angular/core';
+import {ANIMATION_MODULE_TYPE, Component, Injectable, NgZone, RendererFactory2, RendererType2, ViewChild, ɵChangeDetectionScheduler as ChangeDetectionScheduler} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {ɵDomRendererFactory2 as DomRendererFactory2} from '@angular/platform-browser';
 import {InjectableAnimationEngine} from '@angular/platform-browser/animations/src/providers';
@@ -37,7 +37,9 @@ describe('AnimationRenderer', () => {
               (doc: Document, renderer: DomRendererFactory2, zone: NgZone,
                engine: MockAnimationEngine) => {
                 const animationModule = {
-                  ɵcreateEngine: (_: 'animations'|'noop', _2: Document): AnimationEngine => engine,
+                  ɵcreateEngine:
+                      (_: 'animations'|'noop', _2: Document, _3: ChangeDetectionScheduler|null):
+                          AnimationEngine => engine,
                   ɵAnimationEngine: MockAnimationEngine as any,
                   ɵAnimationRenderer: AnimationRenderer,
                   ɵBaseAnimationRenderer: BaseAnimationRenderer,

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

@@ -8,7 +8,7 @@
 
 import {AnimationDriver, NoopAnimationDriver, ɵAnimationEngine as AnimationEngine, ɵAnimationRendererFactory as AnimationRendererFactory, ɵAnimationStyleNormalizer as AnimationStyleNormalizer, ɵWebAnimationsDriver as WebAnimationsDriver, ɵWebAnimationsStyleNormalizer as WebAnimationsStyleNormalizer} from '@angular/animations/browser';
 import {DOCUMENT} from '@angular/common';
-import {ANIMATION_MODULE_TYPE, ApplicationRef, Inject, Injectable, NgZone, OnDestroy, Provider, RendererFactory2} from '@angular/core';
+import {ANIMATION_MODULE_TYPE, inject, Inject, Injectable, NgZone, OnDestroy, Provider, RendererFactory2, ɵChangeDetectionScheduler as ChangeDetectionScheduler} from '@angular/core';
 import {ɵDomRendererFactory2 as DomRendererFactory2} from '@angular/platform-browser';
 
 @Injectable()
@@ -18,8 +18,8 @@ export class InjectableAnimationEngine extends AnimationEngine implements OnDest
   // both have `ngOnDestroy` hooks and `flush()` must be called after all views are destroyed.
   constructor(
       @Inject(DOCUMENT) doc: Document, driver: AnimationDriver,
-      normalizer: AnimationStyleNormalizer, appRef: ApplicationRef) {
-    super(doc, driver, normalizer);
+      normalizer: AnimationStyleNormalizer) {
+    super(doc, driver, normalizer, inject(ChangeDetectionScheduler, {optional: true}));
   }
 
   ngOnDestroy(): void {