From c49b28013a6c017c9afc73bbc00bb4fdcf15c70e Mon Sep 17 00:00:00 2001 From: arturovt Date: Wed, 3 Mar 2021 01:41:06 +0200 Subject: [PATCH] fix(animations): cleanup DOM elements when the root view is removed (#41059) Currently, when importing `BrowserAnimationsModule`, 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. BREAKING CHANGE: DOM elements are now correctly removed when the root view is removed. If you are using SSR and use the app's HTML for rendering, you will need to ensure that you save the HTML to a variable before destorying the app. It is also possible that tests could be accidentally relying on the old behavior by trying to find an element that was not removed in a previous test. If this is the case, the failing tests should be updated to ensure they have proper setup code which initializes elements they rely on. PR Close #41059 --- .../animations/src/providers.ts | 8 +++-- .../test/animation_renderer_spec.ts | 35 ++++++++++++++++++- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/packages/platform-browser/animations/src/providers.ts b/packages/platform-browser/animations/src/providers.ts index ec7438d60d..1cb7f6e151 100644 --- a/packages/platform-browser/animations/src/providers.ts +++ b/packages/platform-browser/animations/src/providers.ts @@ -9,18 +9,22 @@ import {AnimationBuilder} from '@angular/animations'; import {AnimationDriver, ɵAnimationEngine as AnimationEngine, ɵAnimationStyleNormalizer as AnimationStyleNormalizer, ɵCssKeyframesDriver as CssKeyframesDriver, ɵNoopAnimationDriver as NoopAnimationDriver, ɵsupportsWebAnimations as supportsWebAnimations, ɵWebAnimationsDriver as WebAnimationsDriver, ɵWebAnimationsStyleNormalizer as WebAnimationsStyleNormalizer} from '@angular/animations/browser'; import {DOCUMENT} from '@angular/common'; -import {Inject, Injectable, InjectionToken, NgZone, Provider, RendererFactory2} from '@angular/core'; +import {Inject, Injectable, InjectionToken, NgZone, OnDestroy, Provider, RendererFactory2} from '@angular/core'; import {ɵDomRendererFactory2 as DomRendererFactory2} from '@angular/platform-browser'; import {BrowserAnimationBuilder} from './animation_builder'; import {AnimationRendererFactory} from './animation_renderer'; @Injectable() -export class InjectableAnimationEngine extends AnimationEngine { +export class InjectableAnimationEngine extends AnimationEngine implements OnDestroy { constructor( @Inject(DOCUMENT) doc: any, driver: AnimationDriver, normalizer: AnimationStyleNormalizer) { super(doc.body, driver, normalizer); } + + ngOnDestroy(): void { + this.flush(); + } } export function instantiateSupportedAnimationDriver() { diff --git a/packages/platform-browser/animations/test/animation_renderer_spec.ts b/packages/platform-browser/animations/test/animation_renderer_spec.ts index 2fa2ce7aa7..75bc50de60 100644 --- a/packages/platform-browser/animations/test/animation_renderer_spec.ts +++ b/packages/platform-browser/animations/test/animation_renderer_spec.ts @@ -7,10 +7,12 @@ */ import {animate, AnimationPlayer, AnimationTriggerMetadata, state, style, transition, trigger} from '@angular/animations'; import {ɵAnimationEngine as AnimationEngine} from '@angular/animations/browser'; -import {Component, Injectable, NgZone, RendererFactory2, RendererType2, ViewChild} from '@angular/core'; +import {Component, destroyPlatform, Injectable, NgModule, NgZone, RendererFactory2, RendererType2, ViewChild} from '@angular/core'; import {TestBed} from '@angular/core/testing'; +import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; import {BrowserAnimationsModule, ɵAnimationRendererFactory as AnimationRendererFactory, ɵInjectableAnimationEngine as InjectableAnimationEngine} from '@angular/platform-browser/animations'; import {DomRendererFactory2} from '@angular/platform-browser/src/dom/dom_renderer'; +import {onlyInIvy, withBody} from '@angular/private/testing'; import {el} from '../../testing/src/browser_util'; @@ -323,6 +325,37 @@ describe('AnimationRendererFactory', () => { expect(renderer.log).toEqual(['begin', 'end']); }); }); + +onlyInIvy('View Engine uses another mechanism of removing DOM nodes').describe('destroy', () => { + beforeEach(destroyPlatform); + afterEach(destroyPlatform); + + it('should clear bootstrapped component contents', + withBody('
before
after
', async () => { + @Component({selector: 'app-root', template: 'app-root content'}) + class AppComponent { + } + + @NgModule({ + imports: [BrowserAnimationsModule], + declarations: [AppComponent], + bootstrap: [AppComponent] + }) + class AppModule { + } + + const ngModuleRef = await platformBrowserDynamic().bootstrapModule(AppModule); + + const root = document.body.querySelector('app-root')!; + expect(root.textContent).toEqual('app-root content'); + expect(document.body.childNodes.length).toEqual(3); + + ngModuleRef.destroy(); + + expect(document.body.querySelector('app-root')).toBeFalsy(); // host element is removed + expect(document.body.childNodes.length).toEqual(2); // other elements are preserved + })); +}); })(); @Injectable()