Skip to content

Commit

Permalink
fix(animations): cleanup DOM elements when the root view is removed
Browse files Browse the repository at this point in the history
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. It
is 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.
  • Loading branch information
arturovt authored and atscott committed Mar 1, 2021
1 parent 6c05c80 commit 904ec3f
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 3 deletions.
8 changes: 6 additions & 2 deletions packages/platform-browser/animations/src/providers.ts
Expand Up @@ -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() {
Expand Down
Expand Up @@ -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';

Expand Down Expand Up @@ -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('<div>before</div><app-root></app-root><div>after</div>', 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()
Expand Down

0 comments on commit 904ec3f

Please sign in to comment.