From 98d545fafa7fc3b1fb3ae049ce655e33ef9bd423 Mon Sep 17 00:00:00 2001 From: arturovt Date: Sun, 19 Nov 2023 01:04:58 +0200 Subject: [PATCH] fix(animations): cleanup DOM elements when root view is removed with async animations (#53033) 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 --- .../async/src/async_animation_renderer.ts | 22 +++++++++++++++---- .../animations/test/BUILD.bazel | 1 + .../test/animation_renderer_spec.ts | 20 +++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/packages/platform-browser/animations/async/src/async_animation_renderer.ts b/packages/platform-browser/animations/async/src/async_animation_renderer.ts index 767bffa64840d..eac2575ac3099 100644 --- 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|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 index 4b4b56774a861..245b7d4f4b50f 100644 --- 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 index 4a61057ca7d0f..aaef9aae74488 100644 --- 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('
before
after
', 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 }));