Skip to content

Commit c49b280

Browse files
arturovtmhevery
authored andcommitted
fix(animations): cleanup DOM elements when the root view is removed (angular#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 angular#41059
1 parent b555160 commit c49b280

File tree

2 files changed

+40
-3
lines changed

2 files changed

+40
-3
lines changed

packages/platform-browser/animations/src/providers.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,22 @@
99
import {AnimationBuilder} from '@angular/animations';
1010
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';
1111
import {DOCUMENT} from '@angular/common';
12-
import {Inject, Injectable, InjectionToken, NgZone, Provider, RendererFactory2} from '@angular/core';
12+
import {Inject, Injectable, InjectionToken, NgZone, OnDestroy, Provider, RendererFactory2} from '@angular/core';
1313
import {ɵDomRendererFactory2 as DomRendererFactory2} from '@angular/platform-browser';
1414

1515
import {BrowserAnimationBuilder} from './animation_builder';
1616
import {AnimationRendererFactory} from './animation_renderer';
1717

1818
@Injectable()
19-
export class InjectableAnimationEngine extends AnimationEngine {
19+
export class InjectableAnimationEngine extends AnimationEngine implements OnDestroy {
2020
constructor(
2121
@Inject(DOCUMENT) doc: any, driver: AnimationDriver, normalizer: AnimationStyleNormalizer) {
2222
super(doc.body, driver, normalizer);
2323
}
24+
25+
ngOnDestroy(): void {
26+
this.flush();
27+
}
2428
}
2529

2630
export function instantiateSupportedAnimationDriver() {

packages/platform-browser/animations/test/animation_renderer_spec.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@
77
*/
88
import {animate, AnimationPlayer, AnimationTriggerMetadata, state, style, transition, trigger} from '@angular/animations';
99
import {ɵAnimationEngine as AnimationEngine} from '@angular/animations/browser';
10-
import {Component, Injectable, NgZone, RendererFactory2, RendererType2, ViewChild} from '@angular/core';
10+
import {Component, destroyPlatform, Injectable, NgModule, NgZone, RendererFactory2, RendererType2, ViewChild} from '@angular/core';
1111
import {TestBed} from '@angular/core/testing';
12+
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
1213
import {BrowserAnimationsModule, ɵAnimationRendererFactory as AnimationRendererFactory, ɵInjectableAnimationEngine as InjectableAnimationEngine} from '@angular/platform-browser/animations';
1314
import {DomRendererFactory2} from '@angular/platform-browser/src/dom/dom_renderer';
15+
import {onlyInIvy, withBody} from '@angular/private/testing';
1416

1517
import {el} from '../../testing/src/browser_util';
1618

@@ -323,6 +325,37 @@ describe('AnimationRendererFactory', () => {
323325
expect(renderer.log).toEqual(['begin', 'end']);
324326
});
325327
});
328+
329+
onlyInIvy('View Engine uses another mechanism of removing DOM nodes').describe('destroy', () => {
330+
beforeEach(destroyPlatform);
331+
afterEach(destroyPlatform);
332+
333+
it('should clear bootstrapped component contents',
334+
withBody('<div>before</div><app-root></app-root><div>after</div>', async () => {
335+
@Component({selector: 'app-root', template: 'app-root content'})
336+
class AppComponent {
337+
}
338+
339+
@NgModule({
340+
imports: [BrowserAnimationsModule],
341+
declarations: [AppComponent],
342+
bootstrap: [AppComponent]
343+
})
344+
class AppModule {
345+
}
346+
347+
const ngModuleRef = await platformBrowserDynamic().bootstrapModule(AppModule);
348+
349+
const root = document.body.querySelector('app-root')!;
350+
expect(root.textContent).toEqual('app-root content');
351+
expect(document.body.childNodes.length).toEqual(3);
352+
353+
ngModuleRef.destroy();
354+
355+
expect(document.body.querySelector('app-root')).toBeFalsy(); // host element is removed
356+
expect(document.body.childNodes.length).toEqual(2); // other elements are preserved
357+
}));
358+
});
326359
})();
327360

328361
@Injectable()

0 commit comments

Comments
 (0)