fix(animations): cleanup DOM elements when root view is removed with async animations (#53033)

arturovt · thePunderWoman · commit 98d545fafa7f · 2024-01-25T16:32:57.000Z
Currently, when using `provideAnimationsAsync`, Angular uses `AnimationRenderer` as the renderer. When the root view is removed, the `AnimationRenderer` defers the actual work to the `TransitionAnimationEngine` to do this, and the `TransitionAnimationEngine` doesn't actually remove the DOM node, but just calls `markElementAsRemoved()`. The actual DOM node is not removed until `TransitionAnimationEngine` "flushes". Unfortunately, though, that "flush" will never happen, since the root view is being destroyed and there will be no more flushes. This commit adds `flush()` call when the root view is being destroyed. PR Close #53033
diff --git a/packages/platform-browser/animations/async/src/async_animation_renderer.ts b/packages/platform-browser/animations/async/src/async_animation_renderer.ts
@@ -7,14 +7,16 @@
  */
 
 import {ɵAnimationEngine as AnimationEngine, ɵAnimationRenderer as AnimationRenderer, ɵAnimationRendererFactory as AnimationRendererFactory} from '@angular/animations/browser';
-import {inject, NgZone, Renderer2, RendererFactory2, RendererStyleFlags2, RendererType2, ɵAnimationRendererType as AnimationRendererType, ɵChangeDetectionScheduler as ChangeDetectionScheduler, ɵRuntimeError as RuntimeError} from '@angular/core';
+import {inject, Injectable, NgZone, OnDestroy, 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 {
+@Injectable()
+export class AsyncAnimationRendererFactory implements OnDestroy, RendererFactory2 {
   private _rendererFactoryPromise: Promise<AnimationRendererFactory>|null = null;
   private readonly scheduler = inject(ChangeDetectionScheduler, {optional: true});
+  private _engine?: AnimationEngine;
 
   /**
    *
@@ -29,6 +31,17 @@ export class AsyncAnimationRendererFactory implements RendererFactory2 {
         ɵAnimationRendererFactory: typeof AnimationRendererFactory
       }>) {}
 
+  /** @nodoc */
+  ngOnDestroy(): void {
+    // When the root view is removed, the renderer defers the actual work to the
+    // `TransitionAnimationEngine` to do this, and the `TransitionAnimationEngine` doesn't actually
+    // remove the DOM node, but just calls `markElementAsRemoved()`. The actual DOM node is not
+    // removed until `TransitionAnimationEngine` "flushes".
+    // Note: we already flush on destroy within the `InjectableAnimationEngine`. The injectable
+    // engine is not provided when async animations are used.
+    this._engine?.flush();
+  }
+
   /**
    * @internal
    */
@@ -47,8 +60,9 @@ 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, this.scheduler);
-          const rendererFactory = new ɵAnimationRendererFactory(this.delegate, engine, this.zone);
+          this._engine = ɵcreateEngine(this.animationType, this.doc, this.scheduler);
+          const rendererFactory =
+              new ɵAnimationRendererFactory(this.delegate, this._engine, this.zone);
           this.delegate = rendererFactory;
           return rendererFactory;
         });
diff --git a/packages/platform-browser/animations/test/BUILD.bazel b/packages/platform-browser/animations/test/BUILD.bazel
@@ -23,6 +23,7 @@ ts_library(
         "//packages/platform-browser",
         "//packages/platform-browser-dynamic",
         "//packages/platform-browser/animations",
+        "//packages/platform-browser/animations/async",
         "//packages/platform-browser/testing",
         "//packages/private/testing",
         "@npm//rxjs",
diff --git a/packages/platform-browser/animations/test/animation_renderer_spec.ts b/packages/platform-browser/animations/test/animation_renderer_spec.ts
@@ -12,6 +12,7 @@ import {TestBed} from '@angular/core/testing';
 import {bootstrapApplication} from '@angular/platform-browser';
 import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
 import {BrowserAnimationsModule, ɵInjectableAnimationEngine as InjectableAnimationEngine} from '@angular/platform-browser/animations';
+import {provideAnimationsAsync} from '@angular/platform-browser/animations/async';
 import {DomRendererFactory2} from '@angular/platform-browser/src/dom/dom_renderer';
 import {withBody} from '@angular/private/testing';
 
@@ -447,6 +448,25 @@ describe('destroy', () => {
 
        appRef.destroy();
 
+       expect(document.body.querySelector('app-root')).toBeFalsy();  // host element is removed
+       expect(document.body.childNodes.length).toEqual(2);           // other elements are
+     }));
+
+  it('should clear bootstrapped component contents when async animations are used',
+     withBody('<div>before</div><app-root></app-root><div>after</div>', async () => {
+       @Component({selector: 'app-root', template: 'app-root content', standalone: true})
+       class AppComponent {
+       }
+
+       const appRef =
+           await bootstrapApplication(AppComponent, {providers: [provideAnimationsAsync()]});
+
+       const root = document.body.querySelector('app-root')!;
+       expect(root.textContent).toEqual('app-root content');
+       expect(document.body.childNodes.length).toEqual(3);
+
+       appRef.destroy();
+
        expect(document.body.querySelector('app-root')).toBeFalsy();  // host element is removed
        expect(document.body.childNodes.length).toEqual(2);           // other elements are
      }));
