From fab6b39c3de23ae8690ae2a64860933b89cc4691 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Tue, 3 Apr 2018 09:54:58 -0700 Subject: [PATCH] fix(core): inject() should always work in an NgModule injection scope (#23148) Currently the context for inject() is only set when the token is seen for the first time. This has two issues: * It should always be set when injecting from that injector, because a constructor may wish to call inject() directly. * If an NgModuleFactory is .create()'d twice, and an ngInjectableDef token is requested from each of them, the second time will fail. This is because the first injection adds the provider definition and calls the factory, and the provider definitions are shared. The second injector will see the provider definition and call the factory to create an instance, but without setting the correct context for inject(). Fixes angular/material2#10586. PR Close #23148 --- packages/core/src/view/ng_module.ts | 76 +++++++++++------------ packages/core/test/view/ng_module_spec.ts | 13 +++- 2 files changed, 48 insertions(+), 41 deletions(-) diff --git a/packages/core/src/view/ng_module.ts b/packages/core/src/view/ng_module.ts index 339a4368f6..20f4ef060d 100644 --- a/packages/core/src/view/ng_module.ts +++ b/packages/core/src/view/ng_module.ts @@ -75,51 +75,51 @@ export function initNgModule(data: NgModuleData) { export function resolveNgModuleDep( data: NgModuleData, depDef: DepDef, notFoundValue: any = Injector.THROW_IF_NOT_FOUND): any { - if (depDef.flags & DepFlags.Value) { - return depDef.token; - } - if (depDef.flags & DepFlags.Optional) { - notFoundValue = null; - } - if (depDef.flags & DepFlags.SkipSelf) { - return data._parent.get(depDef.token, notFoundValue); - } - const tokenKey = depDef.tokenKey; - switch (tokenKey) { - case InjectorRefTokenKey: - case INJECTORRefTokenKey: - case NgModuleRefTokenKey: - return data; - } - const providerDef = data._def.providersByKey[tokenKey]; - if (providerDef) { - let providerInstance = data._providers[providerDef.index]; - if (providerInstance === undefined) { - providerInstance = data._providers[providerDef.index] = - _createProviderInstance(data, providerDef); + const former = setCurrentInjector(data); + try { + if (depDef.flags & DepFlags.Value) { + return depDef.token; } - return providerInstance === UNDEFINED_VALUE ? undefined : providerInstance; - } else if (depDef.token.ngInjectableDef && targetsModule(data, depDef.token.ngInjectableDef)) { - const injectableDef = depDef.token.ngInjectableDef as InjectableDef; - const key = tokenKey; - const index = data._providers.length; - data._def.providersByKey[depDef.tokenKey] = { - flags: NodeFlags.TypeFactoryProvider | NodeFlags.LazyProvider, - value: injectableDef.factory, - deps: [], index, - token: depDef.token, - }; - const former = setCurrentInjector(data); - try { + if (depDef.flags & DepFlags.Optional) { + notFoundValue = null; + } + if (depDef.flags & DepFlags.SkipSelf) { + return data._parent.get(depDef.token, notFoundValue); + } + const tokenKey = depDef.tokenKey; + switch (tokenKey) { + case InjectorRefTokenKey: + case INJECTORRefTokenKey: + case NgModuleRefTokenKey: + return data; + } + const providerDef = data._def.providersByKey[tokenKey]; + if (providerDef) { + let providerInstance = data._providers[providerDef.index]; + if (providerInstance === undefined) { + providerInstance = data._providers[providerDef.index] = + _createProviderInstance(data, providerDef); + } + return providerInstance === UNDEFINED_VALUE ? undefined : providerInstance; + } else if (depDef.token.ngInjectableDef && targetsModule(data, depDef.token.ngInjectableDef)) { + const injectableDef = depDef.token.ngInjectableDef as InjectableDef; + const key = tokenKey; + const index = data._providers.length; + data._def.providersByKey[depDef.tokenKey] = { + flags: NodeFlags.TypeFactoryProvider | NodeFlags.LazyProvider, + value: injectableDef.factory, + deps: [], index, + token: depDef.token, + }; data._providers[index] = UNDEFINED_VALUE; return ( data._providers[index] = _createProviderInstance(data, data._def.providersByKey[depDef.tokenKey])); - } finally { - setCurrentInjector(former); } + return data._parent.get(depDef.token, notFoundValue); + } finally { + setCurrentInjector(former); } - return data._parent.get(depDef.token, notFoundValue); } function moduleTransitivelyPresent(ngModule: NgModuleData, scope: any): boolean { diff --git a/packages/core/test/view/ng_module_spec.ts b/packages/core/test/view/ng_module_spec.ts index f1433b351b..763f68363a 100644 --- a/packages/core/test/view/ng_module_spec.ts +++ b/packages/core/test/view/ng_module_spec.ts @@ -8,7 +8,7 @@ import {NgModuleRef} from '@angular/core'; import {InjectableDef, defineInjectable} from '@angular/core/src/di/defs'; -import {InjectFlags, Injector, inject} from '@angular/core/src/di/injector'; +import {INJECTOR, InjectFlags, Injector, inject} from '@angular/core/src/di/injector'; import {makePropDecorator} from '@angular/core/src/util/decorators'; import {NgModuleDefinition, NgModuleProviderDef, NodeFlags} from '@angular/core/src/view'; import {moduleDef, moduleProvideDef, resolveNgModuleDep} from '@angular/core/src/view/ng_module'; @@ -88,6 +88,10 @@ class FromChildWithSkipSelfDep { }); } +class UsesInject { + constructor() { inject(INJECTOR); } +} + function makeProviders(classes: any[], modules: any[]): NgModuleDefinition { const providers = classes.map((token, index) => ({ @@ -105,8 +109,8 @@ describe('NgModuleRef_ injector', () => { let ref: NgModuleRef; let childRef: NgModuleRef; beforeEach(() => { - ref = - createNgModuleRef(MyModule, Injector.NULL, [], makeProviders([MyModule, Foo], [MyModule])); + ref = createNgModuleRef( + MyModule, Injector.NULL, [], makeProviders([MyModule, Foo, UsesInject], [MyModule])); childRef = createNgModuleRef( MyChildModule, ref.injector, [], makeProviders([MyChildModule], [MyChildModule])); }); @@ -147,4 +151,7 @@ describe('NgModuleRef_ injector', () => { it('does not inject something not scoped to the module', () => { expect(ref.injector.get(Baz, null)).toBeNull(); }); + + it('injects with the current injector always set', + () => { expect(() => ref.injector.get(UsesInject)).not.toThrow(); }); });