From d52d82d744c6827cfc9c834abc4f7df1db6520e9 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Thu, 25 Oct 2018 11:18:49 -0700 Subject: [PATCH] fix(ivy): injecting optional TemplateRef on element should not throw (#26664) PR Close #26664 --- packages/core/src/linker/template_ref.ts | 2 +- packages/core/src/render3/di.ts | 9 ++- .../src/render3/view_engine_compatibility.ts | 19 +++-- packages/core/test/render3/di_spec.ts | 74 +++++++++++++++---- 4 files changed, 79 insertions(+), 25 deletions(-) diff --git a/packages/core/src/linker/template_ref.ts b/packages/core/src/linker/template_ref.ts index ebc9cdabde..82d116750f 100644 --- a/packages/core/src/linker/template_ref.ts +++ b/packages/core/src/linker/template_ref.ts @@ -55,7 +55,7 @@ export abstract class TemplateRef { /** @internal */ static __NG_ELEMENT_ID__: - () => TemplateRef = () => SWITCH_TEMPLATE_REF_FACTORY(TemplateRef, ElementRef) + () => TemplateRef| null = () => SWITCH_TEMPLATE_REF_FACTORY(TemplateRef, ElementRef) } export const SWITCH_TEMPLATE_REF_FACTORY__POST_R3__ = render3InjectTemplateRef; diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 99de608b01..d967b48e20 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -23,8 +23,6 @@ import {assertNodeOfPossibleTypes} from './node_assert'; import {getPreviousOrParentTNode, getViewData, setTNodeAndViewData} from './state'; import {getParentInjectorIndex, getParentInjectorView, getParentInjectorViewOffset, hasParentInjector, isComponent, stringify} from './util'; - - /** * Defines if the call to `inject` should include `viewProviders` in its resolution. * @@ -300,7 +298,12 @@ export function getOrCreateInjectable( const saveViewData = getViewData(); setTNodeAndViewData(tNode, lViewData); try { - return bloomHash(); + const value = bloomHash(); + if (value == null && !(flags & InjectFlags.Optional)) { + throw new Error(`No provider for ${stringify(token)}`); + } else { + return value; + } } finally { setTNodeAndViewData(savePreviousOrParentTNode, saveViewData); } diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts index fd8c61dd52..79c7e9fea4 100644 --- a/packages/core/src/render3/view_engine_compatibility.ts +++ b/packages/core/src/render3/view_engine_compatibility.ts @@ -77,7 +77,7 @@ let R3TemplateRef: { */ export function injectTemplateRef( TemplateRefToken: typeof ViewEngine_TemplateRef, - ElementRefToken: typeof ViewEngine_ElementRef): ViewEngine_TemplateRef { + ElementRefToken: typeof ViewEngine_ElementRef): ViewEngine_TemplateRef|null { return createTemplateRef( TemplateRefToken, ElementRefToken, getPreviousOrParentTNode(), getViewData()); } @@ -93,7 +93,7 @@ export function injectTemplateRef( */ export function createTemplateRef( TemplateRefToken: typeof ViewEngine_TemplateRef, ElementRefToken: typeof ViewEngine_ElementRef, - hostTNode: TNode, hostView: LViewData): ViewEngine_TemplateRef { + hostTNode: TNode, hostView: LViewData): ViewEngine_TemplateRef|null { if (!R3TemplateRef) { // TODO: Fix class name, should be TemplateRef, but there appears to be a rollup bug R3TemplateRef = class TemplateRef_ extends TemplateRefToken { @@ -122,12 +122,15 @@ export function createTemplateRef( }; } - const hostContainer: LContainer = hostView[hostTNode.index]; - ngDevMode && assertNodeType(hostTNode, TNodeType.Container); - ngDevMode && assertDefined(hostTNode.tViews, 'TView must be allocated'); - return new R3TemplateRef( - hostView, createElementRef(ElementRefToken, hostTNode, hostView), hostTNode.tViews as TView, - getRenderer(), hostContainer[QUERIES], hostTNode.injectorIndex); + if (hostTNode.type === TNodeType.Container) { + const hostContainer: LContainer = hostView[hostTNode.index]; + ngDevMode && assertDefined(hostTNode.tViews, 'TView must be allocated'); + return new R3TemplateRef( + hostView, createElementRef(ElementRefToken, hostTNode, hostView), hostTNode.tViews as TView, + getRenderer(), hostContainer[QUERIES], hostTNode.injectorIndex); + } else { + return null; + } } let R3ViewContainerRef: { diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index 9d2ca97ea9..2675c8595c 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -1181,21 +1181,20 @@ describe('di', () => { }); describe('TemplateRef', () => { - it('should create directive with TemplateRef dependencies', () => { - - class Directive { - value: string; - constructor(public templateRef: TemplateRef) { - this.value = (templateRef.constructor as any).name; - } - static ngDirectiveDef = defineDirective({ - type: Directive, - selectors: [['', 'dir', '']], - factory: () => new Directive(directiveInject(TemplateRef as any)), - exportAs: 'dir' - }); + class Directive { + value: string; + constructor(public templateRef: TemplateRef) { + this.value = (templateRef.constructor as any).name; } + static ngDirectiveDef = defineDirective({ + type: Directive, + selectors: [['', 'dir', '']], + factory: () => new Directive(directiveInject(TemplateRef as any)), + exportAs: 'dir' + }); + } + it('should create directive with TemplateRef dependencies', () => { class DirectiveSameInstance { isSameInstance: boolean; constructor(templateRef: TemplateRef, directive: Directive) { @@ -1233,6 +1232,55 @@ describe('di', () => { expect(fixture.html).toContain('TemplateRef'); expect(fixture.html).toContain('false'); }); + + it('should throw if injected on an element', () => { + /**
*/ + const App = createComponent('app', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + element(0, 'div', ['dir', '']); + } + }, 1, 0, [Directive]); + + expect(() => new ComponentFixture(App)).toThrowError(/No provider for TemplateRef/); + }); + + it('should throw if injected on an ng-container', () => { + /** */ + const App = createComponent('app', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + elementContainerStart(0, ['dir', '']); + elementContainerEnd(); + } + }, 1, 0, [Directive]); + + expect(() => new ComponentFixture(App)).toThrowError(/No provider for TemplateRef/); + }); + + it('should NOT throw if optional and injected on an element', () => { + let dir !: OptionalDirective; + class OptionalDirective { + constructor(@Optional() public templateRef: TemplateRef) {} + + static ngDirectiveDef = defineDirective({ + type: OptionalDirective, + selectors: [['', 'dir', '']], + factory: () => dir = new OptionalDirective( + directiveInject(TemplateRef as any, InjectFlags.Optional)), + exportAs: 'dir' + }); + } + + /**
*/ + const App = createComponent('app', (rf: RenderFlags, ctx: any) => { + if (rf & RenderFlags.Create) { + element(0, 'div', ['dir', '']); + } + }, 1, 0, [OptionalDirective]); + + expect(() => new ComponentFixture(App)).not.toThrow(); + expect(dir.templateRef).toBeNull(); + }); + }); describe('ViewContainerRef', () => {