Skip to content

Commit

Permalink
fix(animations): cleanup DOM elements when root view is removed with …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
arturovt authored and thePunderWoman committed Jan 25, 2024
1 parent 7786e35 commit 98d545f
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
Expand Up @@ -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;

/**
*
Expand All @@ -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
*/
Expand All @@ -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;
});
Expand Down
1 change: 1 addition & 0 deletions packages/platform-browser/animations/test/BUILD.bazel
Expand Up @@ -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",
Expand Down
Expand Up @@ -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';

Expand Down Expand Up @@ -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
}));
Expand Down

0 comments on commit 98d545f

Please sign in to comment.