fix(animations): cleanup DOM elements when the root view is removed (#45143)

arturovt · dylhunn · commit 4eb1ca11f0fb · 2022-06-23T13:52:58.000-07:00
Currently, when importing `BrowserAnimationsModule`, Angular provides `AnimationRendererFactory` as the `RendererFactory2`. The `AnimationRendererFactory` relies on the `AnimationEngine`. The `AnimationEngine` may be created earlier than the `ApplicationRef` (e.g. if it's requested within the `APP_INITIALIZER` before the `ApplicationRef` is created). This means that Angular will add the `AnimationEngine` to `R3Injector.onDestroy` before the `ApplicationRef`. The `R3Injector` will call `ngOnDestroy()` on the `AnimationEngine` before the `ApplicationRef`, which means the `flush()` will be called earlier before views are destroyed. PR Close #45108 PR Close #45143
diff --git a/packages/core/src/application_ref.ts b/packages/core/src/application_ref.ts
@@ -732,8 +732,10 @@ export class ApplicationRef {
 
   /** @internal */
   constructor(
-      private _zone: NgZone, private _injector: Injector, private _exceptionHandler: ErrorHandler,
-      private _initStatus: ApplicationInitStatus) {
+      private _zone: NgZone,
+      private _injector: Injector,
+      private _exceptionHandler: ErrorHandler,
+  ) {
     this._onMicrotaskEmptySubscription = this._zone.onMicrotaskEmpty.subscribe({
       next: () => {
         this._zone.run(() => {
@@ -914,8 +916,9 @@ export class ApplicationRef {
       ComponentRef<C> {
     NG_DEV_MODE && this.warnIfDestroyed();
     const isComponentFactory = componentOrFactory instanceof ComponentFactory;
+    const initStatus = this._injector.get(ApplicationInitStatus);
 
-    if (!this._initStatus.done) {
+    if (!initStatus.done) {
       const standalone = !isComponentFactory && isStandalone(componentOrFactory);
       const errorMessage =
           'Cannot bootstrap as there are still asynchronous initializers running.' +
diff --git a/packages/platform-browser/animations/src/providers.ts b/packages/platform-browser/animations/src/providers.ts
@@ -9,16 +9,20 @@
 import {AnimationBuilder} from '@angular/animations';
 import {AnimationDriver, ɵAnimationEngine as AnimationEngine, ɵAnimationStyleNormalizer as AnimationStyleNormalizer, ɵNoopAnimationDriver as NoopAnimationDriver, ɵWebAnimationsDriver as WebAnimationsDriver, ɵWebAnimationsStyleNormalizer as WebAnimationsStyleNormalizer} from '@angular/animations/browser';
 import {DOCUMENT} from '@angular/common';
-import {ANIMATION_MODULE_TYPE, Inject, Injectable, InjectionToken, NgZone, OnDestroy, Provider, RendererFactory2} from '@angular/core';
+import {ANIMATION_MODULE_TYPE, ApplicationRef, Inject, Injectable, 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 implements OnDestroy {
+  // The `ApplicationRef` is injected here explicitly to force the dependency ordering.
+  // Since the `ApplicationRef` should be created earlier before the `AnimationEngine`, they
+  // both have `ngOnDestroy` hooks and `flush()` must be called after all views are destroyed.
   constructor(
-      @Inject(DOCUMENT) doc: any, driver: AnimationDriver, normalizer: AnimationStyleNormalizer) {
+      @Inject(DOCUMENT) doc: any, driver: AnimationDriver, normalizer: AnimationStyleNormalizer,
+      appRef: ApplicationRef) {
     super(doc.body, driver, normalizer);
   }
 
diff --git a/packages/platform-browser/animations/test/animation_renderer_spec.ts b/packages/platform-browser/animations/test/animation_renderer_spec.ts
@@ -7,8 +7,9 @@
  */
 import {animate, AnimationPlayer, AnimationTriggerMetadata, state, style, transition, trigger} from '@angular/animations';
 import {ɵAnimationEngine as AnimationEngine} from '@angular/animations/browser';
-import {Component, destroyPlatform, Injectable, NgModule, NgZone, RendererFactory2, RendererType2, ViewChild} from '@angular/core';
+import {APP_INITIALIZER, Component, destroyPlatform, importProvidersFrom, Injectable, NgModule, NgZone, RendererFactory2, RendererType2, ViewChild} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
+import {bootstrapApplication} from '@angular/platform-browser';
 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';
@@ -330,7 +331,8 @@ describe('destroy', () => {
   beforeEach(destroyPlatform);
   afterEach(destroyPlatform);
 
-  it('should clear bootstrapped component contents',
+  // See https://github.com/angular/angular/issues/39955
+  it('should clear bootstrapped component contents when the `BrowserAnimationsModule` is imported',
      withBody('<div>before</div><app-root></app-root><div>after</div>', async () => {
        @Component({selector: 'app-root', template: 'app-root content'})
        class AppComponent {
@@ -355,6 +357,99 @@ describe('destroy', () => {
        expect(document.body.querySelector('app-root')).toBeFalsy();  // host element is removed
        expect(document.body.childNodes.length).toEqual(2);           // other elements are preserved
      }));
+
+  // See https://github.com/angular/angular/issues/45108
+  it('should clear bootstrapped component contents when the animation engine is requested during initialization',
+     withBody('<div>before</div><app-root></app-root><div>after</div>', async () => {
+       @Injectable({providedIn: 'root'})
+       class AppService {
+         // The `RendererFactory2` is injected here explicitly because we import the
+         // `BrowserAnimationsModule`. The `RendererFactory2` will be provided with
+         // `AnimationRendererFactory` which relies on the `AnimationEngine`. We want to ensure that
+         // `ApplicationRef` is created earlier before the `AnimationEngine`, because previously the
+         // `AnimationEngine` was created before the `ApplicationRef` (see the link above to the
+         // original issue). The `ApplicationRef` was created after `APP_INITIALIZER` has been run
+         // but before the root module is bootstrapped.
+         constructor(rendererFactory: RendererFactory2) {}
+       }
+
+       @Component({selector: 'app-root', template: 'app-root content'})
+       class AppComponent {
+       }
+
+       @NgModule({
+         imports: [BrowserAnimationsModule],
+         declarations: [AppComponent],
+         bootstrap: [AppComponent],
+         providers: [
+           // The `APP_INITIALIZER` token is requested before the root module is bootstrapped, the
+           // `useFactory` just injects the `AppService` that injects the `RendererFactory2`.
+           {
+             provide: APP_INITIALIZER,
+             useFactory: () => (appService: AppService) => appService,
+             deps: [AppService],
+             multi: true
+           }
+         ]
+       })
+       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
+     }));
+
+  // See https://github.com/angular/angular/issues/45108
+  it('should clear standalone bootstrapped component contents when the animation engine is requested during initialization',
+     withBody('<div>before</div><app-root></app-root><div>after</div>', async () => {
+       @Injectable({providedIn: 'root'})
+       class AppService {
+         // The `RendererFactory2` is injected here explicitly because we import the
+         // `BrowserAnimationsModule`. The `RendererFactory2` will be provided with
+         // `AnimationRendererFactory` which relies on the `AnimationEngine`. We want to ensure
+         // that `ApplicationRef` is created earlier before the `AnimationEngine`, because
+         // previously the `AnimationEngine` was created before the `ApplicationRef` (see the link
+         // above to the original issue). The `ApplicationRef` was created after `APP_INITIALIZER`
+         // has been run but before the root module is bootstrapped.
+         constructor(rendererFactory: RendererFactory2) {}
+       }
+
+       @Component({selector: 'app-root', template: 'app-root content', standalone: true})
+       class AppComponent {
+       }
+
+       const appRef = await bootstrapApplication(AppComponent, {
+         providers: [
+           importProvidersFrom(BrowserAnimationsModule),
+           // The `APP_INITIALIZER` token is requested before the standalone component is
+           // bootstrapped, the `useFactory` just injects the `AppService` that injects the
+           // `RendererFactory2`.
+           {
+             provide: APP_INITIALIZER,
+             useFactory: () => (appService: AppService) => appService,
+             deps: [AppService],
+             multi: true
+           }
+         ]
+       });
+
+       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
+     }));
 });
 })();
 
