From 96770e5c6bac57649e100789246ddc12ab0f34f1 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Thu, 25 Oct 2018 11:35:51 -0700 Subject: [PATCH] fix(ivy): optional dependencies should not throw with module injector (#26763) PR Close #26763 --- packages/core/src/di/r3_injector.ts | 4 +- packages/core/src/render3/di.ts | 5 + packages/core/test/render3/component_spec.ts | 6 +- packages/core/test/render3/di_spec.ts | 123 +++++++++++++------ 4 files changed, 94 insertions(+), 44 deletions(-) diff --git a/packages/core/src/di/r3_injector.ts b/packages/core/src/di/r3_injector.ts index c558860a93..86271b54df 100644 --- a/packages/core/src/di/r3_injector.ts +++ b/packages/core/src/di/r3_injector.ts @@ -178,8 +178,8 @@ export class R3Injector { // Select the next injector based on the Self flag - if self is set, the next injector is // the NullInjector, otherwise it's the parent. - let next = !(flags & InjectFlags.Self) ? this.parent : getNullInjector(); - return this.parent.get(token, notFoundValue); + const nextInjector = !(flags & InjectFlags.Self) ? this.parent : getNullInjector(); + return nextInjector.get(token, notFoundValue); } finally { // Lastly, clean up the state by restoring the previous injector. setCurrentInjector(previousInjector); diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 48cb229e5c..3dbd19d7e6 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -368,6 +368,11 @@ export function getOrCreateInjectable( } } + if (flags & InjectFlags.Optional && notFoundValue === undefined) { + // This must be set or the NullInjector will throw for optional deps + notFoundValue = null; + } + if ((flags & (InjectFlags.Self | InjectFlags.Host)) === 0) { const moduleInjector = lViewData[INJECTOR]; if (moduleInjector) { diff --git a/packages/core/test/render3/component_spec.ts b/packages/core/test/render3/component_spec.ts index ca5170d050..aee02a6a05 100644 --- a/packages/core/test/render3/component_spec.ts +++ b/packages/core/test/render3/component_spec.ts @@ -1565,10 +1565,8 @@ describe('providers', () => { parent: { componentAssertion: () => { expect(directiveInject(String)).toEqual('Module'); - expect(directiveInject(String, InjectFlags.Optional | InjectFlags.Self)) - .toEqual(undefined as any); - expect(directiveInject(String, InjectFlags.Optional | InjectFlags.Host)) - .toEqual(undefined as any); + expect(directiveInject(String, InjectFlags.Optional | InjectFlags.Self)).toBeNull(); + expect(directiveInject(String, InjectFlags.Optional | InjectFlags.Host)).toBeNull(); } } }); diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index d0fca9879a..6d832a52ad 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Attribute, ChangeDetectorRef, ElementRef, Host, InjectFlags, Injector, Optional, Renderer2, Self, SkipSelf, TemplateRef, ViewContainerRef, defineInjectable} from '@angular/core'; +import {Attribute, ChangeDetectorRef, ElementRef, Host, InjectFlags, Injector, Optional, Renderer2, Self, SkipSelf, TemplateRef, ViewContainerRef, createInjector, defineInjectable, defineInjector} from '@angular/core'; import {RenderFlags} from '@angular/core/src/render3/interfaces/definition'; import {defineComponent} from '../../src/render3/definition'; @@ -857,8 +857,8 @@ describe('di', () => { }); } - it('should not throw if dependency is @Optional', () => { - let dirA: DirA; + describe('Optional', () => { + let dirA: DirA|null = null; class DirA { constructor(@Optional() public dirB: DirB|null) {} @@ -870,47 +870,94 @@ describe('di', () => { }); } - /**
*/ - const App = createComponent('app', function(rf: RenderFlags, ctx: any) { - if (rf & RenderFlags.Create) { - element(0, 'div', ['dirA', '']); - } - }, 1, 0, [DirA, DirB]); + beforeEach(() => dirA = null); - expect(() => { - new ComponentFixture(App); + it('should not throw if dependency is @Optional (limp mode)', () => { + + /**
*/ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + element(0, 'div', ['dirA', '']); + } + }, 1, 0, [DirA, DirB]); + + expect(() => { new ComponentFixture(App); }).not.toThrow(); expect(dirA !.dirB).toEqual(null); - }).not.toThrow(); - }); + }); - it('should not throw if dependency is @Optional but defined elsewhere', () => { - let dirA: DirA; - - class DirA { - constructor(@Optional() public dirB: DirB|null) {} - - static ngDirectiveDef = defineDirective({ - type: DirA, - selectors: [['', 'dirA', '']], - factory: () => dirA = new DirA(directiveInject(DirB, InjectFlags.Optional)) - }); - } - - /** - *
- *
- */ - const App = createComponent('app', function(rf: RenderFlags, ctx: any) { - if (rf & RenderFlags.Create) { - element(0, 'div', ['dirB', '']); - element(1, 'div', ['dirA', '']); + it('should not throw if dependency is @Optional (module injector)', () => { + class SomeModule { + static ngInjectorDef = defineInjector({factory: () => new SomeModule()}); } - }, 2, 0, [DirA, DirB]); - expect(() => { - new ComponentFixture(App); + /**
*/ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + element(0, 'div', ['dirA', '']); + } + }, 1, 0, [DirA, DirB]); + + expect(() => { + const injector = createInjector(SomeModule); + new ComponentFixture(App, {injector}); + }).not.toThrow(); expect(dirA !.dirB).toEqual(null); - }).not.toThrow(); + }); + + it('should return null if @Optional dependency has @Self flag', () => { + let dirC !: DirC; + + class DirC { + constructor(@Optional() @Self() public dirB: DirB|null) {} + + static ngDirectiveDef = defineDirective({ + type: DirC, + selectors: [['', 'dirC', '']], + factory: () => dirC = + new DirC(directiveInject(DirB, InjectFlags.Optional|InjectFlags.Self)) + }); + } + + /**
*/ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + element(0, 'div', ['dirC', '']); + } + }, 1, 0, [DirC, DirB]); + + expect(() => { new ComponentFixture(App); }).not.toThrow(); + expect(dirC !.dirB).toEqual(null); + }); + + it('should not throw if dependency is @Optional but defined elsewhere', () => { + let dirA: DirA; + + class DirA { + constructor(@Optional() public dirB: DirB|null) {} + + static ngDirectiveDef = defineDirective({ + type: DirA, + selectors: [['', 'dirA', '']], + factory: () => dirA = new DirA(directiveInject(DirB, InjectFlags.Optional)) + }); + } + + /** + *
+ *
+ */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + element(0, 'div', ['dirB', '']); + element(1, 'div', ['dirA', '']); + } + }, 2, 0, [DirA, DirB]); + + expect(() => { + new ComponentFixture(App); + expect(dirA !.dirB).toEqual(null); + }).not.toThrow(); + }); }); it('should skip the current node with @SkipSelf', () => {