fix(compiler): correctly instantiate eager providers that are used via `Injector.get` (#19558)

Closes #15501

PR Close #19558

packages/core/src/view/ng_module.ts

@@ -13,7 +13,7 @@ import {NgModuleRef} from '../linker/ng_module_factory';
 import {DepDef, DepFlags, NgModuleData, NgModuleDefinition, NgModuleProviderDef, NodeFlags} from './types';
 import {splitDepsDsl, tokenKey} from './util';
 
-const NOT_CREATED = new Object();
+const UNDEFINED_VALUE = new Object();
 
 const InjectorRefTokenKey = tokenKey(Injector);
 const NgModuleRefTokenKey = tokenKey(NgModuleRef);
@@ -53,8 +53,9 @@ export function initNgModule(data: NgModuleData) {
   const providers = data._providers = new Array(def.providers.length);
   for (let i = 0; i < def.providers.length; i++) {
     const provDef = def.providers[i];
-    providers[i] = provDef.flags & NodeFlags.LazyProvider ? NOT_CREATED :
+    if (!(provDef.flags & NodeFlags.LazyProvider)) {
-                                                            _createProviderInstance(data, provDef);
+      providers[i] = _createProviderInstance(data, provDef);
+    }
   }
 }
 
@@ -78,27 +79,33 @@ export function resolveNgModuleDep(
   const providerDef = data._def.providersByKey[tokenKey];
   if (providerDef) {
     let providerInstance = data._providers[providerDef.index];
-    if (providerInstance === NOT_CREATED) {
+    if (providerInstance === undefined) {
       providerInstance = data._providers[providerDef.index] =
           _createProviderInstance(data, providerDef);
     }
-    return providerInstance;
+    return providerInstance === UNDEFINED_VALUE ? undefined : providerInstance;
   }
   return data._parent.get(depDef.token, notFoundValue);
 }
 
 
 function _createProviderInstance(ngModule: NgModuleData, providerDef: NgModuleProviderDef): any {
+  let injectable: any;
   switch (providerDef.flags & NodeFlags.Types) {
     case NodeFlags.TypeClassProvider:
-      return _createClass(ngModule, providerDef.value, providerDef.deps);
+      injectable = _createClass(ngModule, providerDef.value, providerDef.deps);
+      break;
     case NodeFlags.TypeFactoryProvider:
-      return _callFactory(ngModule, providerDef.value, providerDef.deps);
+      injectable = _callFactory(ngModule, providerDef.value, providerDef.deps);
+      break;
     case NodeFlags.TypeUseExistingProvider:
-      return resolveNgModuleDep(ngModule, providerDef.deps[0]);
+      injectable = resolveNgModuleDep(ngModule, providerDef.deps[0]);
+      break;
     case NodeFlags.TypeValueProvider:
-      return providerDef.value;
+      injectable = providerDef.value;
+      break;
   }
+  return injectable === undefined ? UNDEFINED_VALUE : injectable;
 }
 
 function _createClass(ngModule: NgModuleData, ctor: any, deps: DepDef[]): any {
@@ -151,7 +158,7 @@ export function callNgModuleLifecycle(ngModule: NgModuleData, lifecycles: NodeFl
     const provDef = def.providers[i];
     if (provDef.flags & NodeFlags.OnDestroy) {
       const instance = ngModule._providers[i];
-      if (instance && instance !== NOT_CREATED) {
+      if (instance && instance !== UNDEFINED_VALUE) {
         instance.ngOnDestroy();
       }
     }

packages/core/src/view/provider.ts

@@ -25,8 +25,6 @@ const TemplateRefTokenKey = tokenKey(TemplateRef);
 const ChangeDetectorRefTokenKey = tokenKey(ChangeDetectorRef);
 const InjectorRefTokenKey = tokenKey(Injector);
 
-const NOT_CREATED = new Object();
-
 export function directiveDef(
     checkIndex: number, flags: NodeFlags,
     matchedQueries: null | [string | number, QueryValueType][], childCount: number, ctor: any,
@@ -111,7 +109,7 @@ export function _def(
 }
 
 export function createProviderInstance(view: ViewData, def: NodeDef): any {
-  return def.flags & NodeFlags.LazyProvider ? NOT_CREATED : _createProviderInstance(view, def);
+  return _createProviderInstance(view, def);
 }
 
 export function createPipeInstance(view: ViewData, def: NodeDef): any {
@@ -378,9 +376,10 @@ export function resolveDep(
               (allowPrivateServices ? elDef.element !.allProviders :
                                       elDef.element !.publicProviders) ![tokenKey];
           if (providerDef) {
-            const providerData = asProviderData(view, providerDef.nodeIndex);
+            let providerData = asProviderData(view, providerDef.nodeIndex);
-            if (providerData.instance === NOT_CREATED) {
+            if (!providerData) {
-              providerData.instance = _createProviderInstance(view, providerDef);
+              providerData = {instance: _createProviderInstance(view, providerDef)};
+              view.nodes[providerDef.nodeIndex] = providerData as any;
             }
             return providerData.instance;
           }
@@ -487,8 +486,12 @@ function callElementProvidersLifecycles(view: ViewData, elDef: NodeDef, lifecycl
 }
 
 function callProviderLifecycles(view: ViewData, index: number, lifecycles: NodeFlags) {
-  const provider = asProviderData(view, index).instance;
+  const providerData = asProviderData(view, index);
-  if (provider === NOT_CREATED) {
+  if (!providerData) {
+    return;
+  }
+  const provider = providerData.instance;
+  if (!provider) {
     return;
   }
   Services.setCurrentNode(view, index);

packages/core/src/view/view.ts

@@ -284,8 +284,11 @@ function createViewNodes(view: ViewData) {
       case NodeFlags.TypeFactoryProvider:
       case NodeFlags.TypeUseExistingProvider:
       case NodeFlags.TypeValueProvider: {
+        nodeData = nodes[i];
+        if (!nodeData && !(nodeDef.flags & NodeFlags.LazyProvider)) {
           const instance = createProviderInstance(view, nodeDef);
           nodeData = <ProviderData>{instance};
+        }
         break;
       }
       case NodeFlags.TypePipe: {
@@ -294,11 +297,14 @@ function createViewNodes(view: ViewData) {
         break;
       }
       case NodeFlags.TypeDirective: {
+        nodeData = nodes[i];
+        if (!nodeData) {
           const instance = createDirectiveInstance(view, nodeDef);
           nodeData = <ProviderData>{instance};
+        }
         if (nodeDef.flags & NodeFlags.Component) {
           const compView = asElementData(view, nodeDef.parent !.nodeIndex).componentView;
-          initView(compView, instance, instance);
+          initView(compView, nodeData.instance, nodeData.instance);
         }
         break;
       }

packages/core/test/linker/ng_module_integration_spec.ts

@@ -787,6 +787,22 @@ function declareTests({useJit}: {useJit: boolean}) {
         expect(child.get(Injector)).toBe(child);
       });
 
+      it('should provide undefined', () => {
+        let factoryCounter = 0;
+
+        const injector = createInjector([{
+          provide: 'token',
+          useFactory: () => {
+            factoryCounter++;
+            return undefined;
+          }
+        }]);
+
+        expect(injector.get('token')).toBeUndefined();
+        expect(injector.get('token')).toBeUndefined();
+        expect(factoryCounter).toBe(1);
+      });
+
       describe('injecting lazy providers into an eager provider via Injector.get', () => {
 
         it('should inject providers that were declared before it', () => {
@@ -828,6 +844,47 @@ function declareTests({useJit}: {useJit: boolean}) {
         });
       });
 
+      describe('injecting eager providers into an eager provider via Injector.get', () => {
+
+        it('should inject providers that were declared before it', () => {
+          @NgModule({
+            providers: [
+              {provide: 'eager1', useFactory: () => 'v1'},
+              {
+                provide: 'eager2',
+                useFactory: (i: Injector) => `v2: ${i.get('eager1')}`,
+                deps: [Injector]
+              },
+            ]
+          })
+          class MyModule {
+            // NgModule is eager, which makes all of its deps eager
+            constructor(@Inject('eager1') eager1: any, @Inject('eager2') eager2: any) {}
+          }
+
+          expect(createModule(MyModule).injector.get('eager2')).toBe('v2: v1');
+        });
+
+        it('should inject providers that were declared after it', () => {
+          @NgModule({
+            providers: [
+              {
+                provide: 'eager1',
+                useFactory: (i: Injector) => `v1: ${i.get('eager2')}`,
+                deps: [Injector]
+              },
+              {provide: 'eager2', useFactory: () => 'v2'},
+            ]
+          })
+          class MyModule {
+            // NgModule is eager, which makes all of its deps eager
+            constructor(@Inject('eager1') eager1: any, @Inject('eager2') eager2: any) {}
+          }
+
+          expect(createModule(MyModule).injector.get('eager1')).toBe('v1: v2');
+        });
+      });
+
       it('should throw when no provider defined', () => {
         const injector = createInjector([]);
         expect(() => injector.get('NonExisting'))

packages/core/test/linker/view_injector_integration_spec.ts

@@ -342,6 +342,22 @@ export function main() {
         expect(created).toBe(true);
       });
 
+      it('should provide undefined', () => {
+        let factoryCounter = 0;
+
+        const el = createComponent('', [{
+                                     provide: 'token',
+                                     useFactory: () => {
+                                       factoryCounter++;
+                                       return undefined;
+                                     }
+                                   }]);
+
+        expect(el.injector.get('token')).toBeUndefined();
+        expect(el.injector.get('token')).toBeUndefined();
+        expect(factoryCounter).toBe(1);
+      });
+
       describe('injecting lazy providers into an eager provider via Injector.get', () => {
 
         it('should inject providers that were declared before it', () => {
@@ -389,6 +405,52 @@ export function main() {
         });
       });
 
+      describe('injecting eager providers into an eager provider via Injector.get', () => {
+        it('should inject providers that were declared before it', () => {
+          @Component({
+            template: '',
+            providers: [
+              {provide: 'eager1', useFactory: () => 'v1'},
+              {
+                provide: 'eager2',
+                useFactory: (i: Injector) => `v2: ${i.get('eager1')}`,
+                deps: [Injector]
+              },
+            ]
+          })
+          class MyComp {
+            // Component is eager, which makes all of its deps eager
+            constructor(@Inject('eager1') eager1: any, @Inject('eager2') eager2: any) {}
+          }
+
+          const ctx =
+              TestBed.configureTestingModule({declarations: [MyComp]}).createComponent(MyComp);
+          expect(ctx.debugElement.injector.get('eager2')).toBe('v2: v1');
+        });
+
+        it('should inject providers that were declared after it', () => {
+          @Component({
+            template: '',
+            providers: [
+              {
+                provide: 'eager1',
+                useFactory: (i: Injector) => `v1: ${i.get('eager2')}`,
+                deps: [Injector]
+              },
+              {provide: 'eager2', useFactory: () => 'v2'},
+            ]
+          })
+          class MyComp {
+            // Component is eager, which makes all of its deps eager
+            constructor(@Inject('eager1') eager1: any, @Inject('eager2') eager2: any) {}
+          }
+
+          const ctx =
+              TestBed.configureTestingModule({declarations: [MyComp]}).createComponent(MyComp);
+          expect(ctx.debugElement.injector.get('eager1')).toBe('v1: v2');
+        });
+      });
+
       it('should allow injecting lazy providers via Injector.get from an eager provider that is declared earlier',
          () => {
            @Component({providers: [{provide: 'a', useFactory: () => 'aValue'}], template: ''})

packages/platform-webworker/test/web_workers/worker/renderer_v2_integration_spec.ts

@@ -12,7 +12,7 @@ import {platformBrowserDynamicTesting} from '@angular/platform-browser-dynamic/t
 import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
 import {DomRendererFactory2} from '@angular/platform-browser/src/dom/dom_renderer';
 import {BrowserTestingModule} from '@angular/platform-browser/testing';
-import {dispatchEvent} from '@angular/platform-browser/testing/src/browser_util';
+import {browserDetection, dispatchEvent} from '@angular/platform-browser/testing/src/browser_util';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 
 import {ClientMessageBrokerFactory} from '../../../src/web_workers/shared/client_message_broker';
@@ -29,6 +29,10 @@ export function main() {
   describe('Web Worker Renderer v2', () => {
     // Don't run on server...
     if (!getDOM().supportsDOMEvents()) return;
+    // TODO(tbosch): investigate why this is failing on iOS7 for unrelated reasons
+    // Note: it's hard to debug this as SauceLabs starts with iOS8. Maybe drop
+    // iOS7 alltogether?
+    if (browserDetection.isIOS7) return;
 
     let uiRenderStore: RenderStore;
     let wwRenderStore: RenderStore;