fix(core): call ngOnDestroy for tree-shakeable providers (#28943)

JoostK · IgorMinar · commit 30b04424a343 · 2019-04-05T09:40:00.000-07:00
Prior to this change, any provider that was independently resolved using its InjectableDef would not be considered when destroying the module it was requested from. This commit provides a fix for this issue by storing the resolved provider in the module's list of provider definitions. Fixes #28927 PR Close #28943
diff --git a/packages/core/src/view/ng_module.ts b/packages/core/src/view/ng_module.ts
@@ -109,7 +109,7 @@ export function resolveNgModuleDep(
     } else if (
         (injectableDef = getInjectableDef(depDef.token)) && targetsModule(data, injectableDef)) {
       const index = data._providers.length;
-      data._def.providersByKey[depDef.tokenKey] = {
+      data._def.providers[index] = data._def.providersByKey[depDef.tokenKey] = {
         flags: NodeFlags.TypeFactoryProvider | NodeFlags.LazyProvider,
         value: injectableDef.factory,
         deps: [], index,
diff --git a/packages/core/test/view/ng_module_spec.ts b/packages/core/test/view/ng_module_spec.ts
@@ -200,6 +200,28 @@ describe('NgModuleRef_ injector', () => {
     expect(Service.destroyed).toBe(1);
   });
 
+  it('calls ngOnDestroy on scoped providers', () => {
+    class Module {}
+
+    class Service {
+      static destroyed = 0;
+
+      ngOnDestroy(): void { Service.destroyed++; }
+
+      static ngInjectableDef: InjectableDef<Service> = defineInjectable({
+        factory: () => new Service(),
+        providedIn: 'root',
+      });
+    }
+
+    const ref = createNgModuleRef(Module, Injector.NULL, [], makeFactoryProviders([], [Module]));
+
+    expect(ref.injector.get(Service)).toBeDefined();
+    expect(Service.destroyed).toBe(0);
+    ref.destroy();
+    expect(Service.destroyed).toBe(1);
+  });
+
   it('only calls ngOnDestroy once per instance', () => {
     class Module {}
 
