fix(ivy): destroy injector when module is destroyed (angular#27793)

Destroys the module's injector when an `NgModule` is destroyed which in turn calls the `ngOnDestroy` methods on the instantiated providers.

This PR resolves FW-739.

PR Close angular#27793

Author: crisbeto

packages/core/src/di/r3_injector.ts

@@ -102,7 +102,8 @@ export class R3Injector {
   /**
    * Flag indicating that this injector was previously destroyed.
    */
-  private destroyed = false;
+  get destroyed(): boolean { return this._destroyed; }
+  private _destroyed = false;
 
   constructor(
       def: InjectorType<any>, additionalProviders: StaticProvider[]|null,
@@ -138,7 +139,7 @@ export class R3Injector {
     this.assertNotDestroyed();
 
     // Set destroyed = true first, in case lifecycle hooks re-enter destroy().
-    this.destroyed = true;
+    this._destroyed = true;
     try {
       // Call all the lifecycle hooks.
       this.onDestroy.forEach(service => service.ngOnDestroy());
@@ -189,7 +190,7 @@ export class R3Injector {
   }
 
   private assertNotDestroyed(): void {
-    if (this.destroyed) {
+    if (this._destroyed) {
       throw new Error('Injector has already been destroyed.');
     }
   }

packages/core/src/render3/component_ref.ts

@@ -273,7 +273,7 @@ export class ComponentRef<T> extends viewEngine_ComponentRef<T> {
     ngDevMode && assertDefined(this.destroyCbs, 'NgModule already destroyed');
     this.destroyCbs !.forEach(fn => fn());
     this.destroyCbs = null;
-    this.hostView.destroy();
+    !this.hostView.destroyed && this.hostView.destroy();
   }
   onDestroy(callback: () => void): void {
     ngDevMode && assertDefined(this.destroyCbs, 'NgModule already destroyed');

packages/core/src/render3/ng_module_ref.ts

@@ -9,7 +9,7 @@
 import {INJECTOR, Injector} from '../di/injector';
 import {InjectFlags} from '../di/interface/injector';
 import {StaticProvider} from '../di/interface/provider';
-import {createInjector} from '../di/r3_injector';
+import {R3Injector, createInjector} from '../di/r3_injector';
 import {Type} from '../interface/type';
 import {ComponentFactoryResolver as viewEngine_ComponentFactoryResolver} from '../linker/component_factory_resolver';
 import {InternalNgModuleRef, NgModuleFactory as viewEngine_NgModuleFactory, NgModuleRef as viewEngine_NgModuleRef} from '../linker/ng_module_factory';
@@ -32,7 +32,7 @@ export class NgModuleRef<T> extends viewEngine_NgModuleRef<T> implements Interna
   // tslint:disable-next-line:require-internal-with-underscore
   _bootstrapComponents: Type<any>[] = [];
   // tslint:disable-next-line:require-internal-with-underscore
-  _r3Injector: Injector;
+  _r3Injector: R3Injector;
   injector: Injector = this;
   instance: T;
   destroyCbs: (() => void)[]|null = [];
@@ -52,7 +52,7 @@ export class NgModuleRef<T> extends viewEngine_NgModuleRef<T> implements Interna
       },
       COMPONENT_FACTORY_RESOLVER
     ];
-    this._r3Injector = createInjector(ngModuleType, _parent, additionalProviders);
+    this._r3Injector = createInjector(ngModuleType, _parent, additionalProviders) as R3Injector;
     this.instance = this.get(ngModuleType);
   }
 
@@ -70,6 +70,8 @@ export class NgModuleRef<T> extends viewEngine_NgModuleRef<T> implements Interna
 
   destroy(): void {
     ngDevMode && assertDefined(this.destroyCbs, 'NgModule already destroyed');
+    const injector = this._r3Injector;
+    !injector.destroyed && injector.destroy();
     this.destroyCbs !.forEach(fn => fn());
     this.destroyCbs = null;
   }

packages/core/test/linker/ng_module_integration_spec.ts

@@ -1045,55 +1045,53 @@ function declareTests(config?: {useJit: boolean}) {
           expect(created).toBe(false);
         });
 
-        fixmeIvy('FW-739: TestBed: destroy on NgModuleRef is not being called')
-            .it('should support ngOnDestroy on any provider', () => {
-              let destroyed = false;
+        it('should support ngOnDestroy on any provider', () => {
+          let destroyed = false;
 
-              class SomeInjectable {
-                ngOnDestroy() { destroyed = true; }
-              }
+          class SomeInjectable {
+            ngOnDestroy() { destroyed = true; }
+          }
 
-              @NgModule({providers: [SomeInjectable]})
-              class SomeModule {
-                // Inject SomeInjectable to make it eager...
-                constructor(i: SomeInjectable) {}
-              }
+          @NgModule({providers: [SomeInjectable]})
+          class SomeModule {
+            // Inject SomeInjectable to make it eager...
+            constructor(i: SomeInjectable) {}
+          }
 
-              const moduleRef = createModule(SomeModule);
-              expect(destroyed).toBe(false);
-              moduleRef.destroy();
-              expect(destroyed).toBe(true);
-            });
+          const moduleRef = createModule(SomeModule);
+          expect(destroyed).toBe(false);
+          moduleRef.destroy();
+          expect(destroyed).toBe(true);
+        });
 
-        fixmeIvy('FW-739: TestBed: destroy on NgModuleRef is not being called')
-            .it('should support ngOnDestroy for lazy providers', () => {
-              let created = false;
-              let destroyed = false;
+        it('should support ngOnDestroy for lazy providers', () => {
+          let created = false;
+          let destroyed = false;
 
-              class SomeInjectable {
-                constructor() { created = true; }
-                ngOnDestroy() { destroyed = true; }
-              }
+          class SomeInjectable {
+            constructor() { created = true; }
+            ngOnDestroy() { destroyed = true; }
+          }
 
-              @NgModule({providers: [SomeInjectable]})
-              class SomeModule {
-              }
+          @NgModule({providers: [SomeInjectable]})
+          class SomeModule {
+          }
 
-              let moduleRef = createModule(SomeModule);
-              expect(created).toBe(false);
-              expect(destroyed).toBe(false);
+          let moduleRef = createModule(SomeModule);
+          expect(created).toBe(false);
+          expect(destroyed).toBe(false);
 
-              // no error if the provider was not yet created
-              moduleRef.destroy();
-              expect(created).toBe(false);
-              expect(destroyed).toBe(false);
+          // no error if the provider was not yet created
+          moduleRef.destroy();
+          expect(created).toBe(false);
+          expect(destroyed).toBe(false);
 
-              moduleRef = createModule(SomeModule);
-              moduleRef.injector.get(SomeInjectable);
-              expect(created).toBe(true);
-              moduleRef.destroy();
-              expect(destroyed).toBe(true);
-            });
+          moduleRef = createModule(SomeModule);
+          moduleRef.injector.get(SomeInjectable);
+          expect(created).toBe(true);
+          moduleRef.destroy();
+          expect(destroyed).toBe(true);
+        });
       });
 
       describe('imported and exported modules', () => {

packages/upgrade/src/common/downgrade_component_adapter.ts

@@ -213,15 +213,15 @@ export class DowngradeComponentAdapter {
   }
 
   registerCleanup() {
+    const testabilityRegistry = this.componentRef.injector.get(TestabilityRegistry);
     const destroyComponentRef = this.wrapCallback(() => this.componentRef.destroy());
     let destroyed = false;
 
     this.element.on !('$destroy', () => this.componentScope.$destroy());
     this.componentScope.$on('$destroy', () => {
       if (!destroyed) {
         destroyed = true;
-        this.componentRef.injector.get(TestabilityRegistry)
-            .unregisterApplication(this.componentRef.location.nativeElement);
+        testabilityRegistry.unregisterApplication(this.componentRef.location.nativeElement);
         destroyComponentRef();
       }
     });