From db77d8dc9247a0a88e3afc598b8035cdf9cf85f0 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Mon, 23 Apr 2018 18:24:40 -0700 Subject: [PATCH] feat(ivy): support injection flags at runtime (#23518) PR Close #23518 --- packages/core/src/di/injector.ts | 11 +- packages/core/src/render3/di.ts | 49 ++- .../bundling/todo/bundle.golden_symbols.json | 3 + packages/core/test/render3/di_spec.ts | 313 ++++++++++++++++-- 4 files changed, 324 insertions(+), 52 deletions(-) diff --git a/packages/core/src/di/injector.ts b/packages/core/src/di/injector.ts index 87d829e2ba..5cbd618846 100644 --- a/packages/core/src/di/injector.ts +++ b/packages/core/src/di/injector.ts @@ -413,19 +413,19 @@ function getClosureSafeProperty(objWithPropertyToExtract: T): string { * Injection flags for DI. */ export const enum InjectFlags { - Default = 0, + Default = 0b0000, /** * Specifies that an injector should retrieve a dependency from any injector until reaching the * host element of the current component. (Only used with Element Injector) */ - Host = 1 << 0, + Host = 0b0001, /** Don't descend into ancestors of the node requesting injection. */ - Self = 1 << 1, + Self = 0b0010, /** Skip the node that is requesting injection. */ - SkipSelf = 1 << 2, + SkipSelf = 0b0100, /** Inject `defaultValue` instead if token not found. */ - Optional = 1 << 3, + Optional = 0b1000, } /** @@ -467,6 +467,7 @@ export function inject(token: Type| InjectionToken, flags = InjectFlags return injectableDef.value === undefined ? injectableDef.value = injectableDef.factory() : injectableDef.value; } + if (flags & InjectFlags.Optional) return null; throw new Error(`Injector: NOT_FOUND [${stringify(token)}]`); } else { return _currentInjector.get(token, flags & InjectFlags.Optional ? null : undefined, flags); diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index c1c76c53fa..c82de9358f 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -178,7 +178,8 @@ export function diPublic(def: DirectiveDef): void { * @returns The instance found */ export function directiveInject(token: Type): T; -export function directiveInject(token: Type, flags?: InjectFlags): T|null; +export function directiveInject(token: Type, flags: InjectFlags.Optional): T|null; +export function directiveInject(token: Type, flags: InjectFlags): T; export function directiveInject(token: Type, flags = InjectFlags.Default): T|null { return getOrCreateInjectable(getOrCreateNodeInjector(), token, flags); } @@ -329,8 +330,8 @@ function getClosestComponentAncestor(node: LViewNode | LElementNode): LElementNo * @param flags Injection flags (e.g. CheckParent) * @returns The instance found */ -export function getOrCreateInjectable(di: LInjector, token: Type, flags?: InjectFlags): T| - null { +export function getOrCreateInjectable( + di: LInjector, token: Type, flags: InjectFlags = InjectFlags.Default): T|null { const bloomHash = bloomHashBit(token); // If the token has a bloom hash, then it is a directive that is public to the injection system @@ -349,7 +350,7 @@ export function getOrCreateInjectable(di: LInjector, token: Type, flags?: while (injector) { // Get the closest potential matching injector (upwards in the injector tree) that // *potentially* has the token. - injector = bloomFindPossibleInjector(injector, bloomHash); + injector = bloomFindPossibleInjector(injector, bloomHash, flags); // If no injector is found, we *know* that there is no ancestor injector that contains the // token, so we abort. @@ -360,11 +361,11 @@ export function getOrCreateInjectable(di: LInjector, token: Type, flags?: // At this point, we have an injector which *may* contain the token, so we step through the // directives associated with the injector's corresponding node to get the directive instance. const node = injector.node; - const flags = node.tNode !.flags; - const count = flags & TNodeFlags.DirectiveCountMask; + const nodeFlags = node.tNode !.flags; + const count = nodeFlags & TNodeFlags.DirectiveCountMask; if (count !== 0) { - const start = flags >> TNodeFlags.DirectiveStartingIndexShift; + const start = nodeFlags >> TNodeFlags.DirectiveStartingIndexShift; const end = start + count; const defs = node.view.tView.directives !; @@ -385,15 +386,19 @@ export function getOrCreateInjectable(di: LInjector, token: Type, flags?: return instance; } - // The def wasn't found anywhere on this node, so it might be a false positive. - // Traverse up the tree and continue searching. - injector = injector.parent; + // The def wasn't found anywhere on this node, so it was a false positive. + // If flags permit, traverse up the tree and continue searching. + if (flags & InjectFlags.Self || flags & InjectFlags.Host && !sameHostView(injector)) { + injector = null; + } else { + injector = injector.parent; + } } } // No directive was found for the given token. - // TODO: implement optional, check-self, and check-parent. - throw new Error('Implement'); + if (flags & InjectFlags.Optional) return null; + throw new Error(`Injector: NOT_FOUND [${stringify(token)}]`); } function searchMatchesQueuedForCreation(node: LNode, token: any): T|null { @@ -443,10 +448,11 @@ function bloomHashBit(type: Type): number|null { * * @param injector The starting node injector to check * @param bloomBit The bit to check in each injector's bloom filter + * @param flags The injection flags for this injection site (e.g. Optional or SkipSelf) * @returns An injector that might have the directive */ -export function bloomFindPossibleInjector(startInjector: LInjector, bloomBit: number): LInjector| - null { +export function bloomFindPossibleInjector( + startInjector: LInjector, bloomBit: number, flags: InjectFlags): LInjector|null { // Create a mask that targets the specific bit associated with the directive we're looking for. // JS bit operations are 32 bits, so this will be a number between 2^0 and 2^31, corresponding // to bit positions 0 - 31 in a 32 bit integer. @@ -454,7 +460,8 @@ export function bloomFindPossibleInjector(startInjector: LInjector, bloomBit: nu // Traverse up the injector tree until we find a potential match or until we know there *isn't* a // match. - let injector: LInjector|null = startInjector; + let injector: LInjector|null = + flags & InjectFlags.SkipSelf ? startInjector.parent ! : startInjector; while (injector) { // Our bloom filter size is 256 bits, which is eight 32-bit bloom filter buckets: // bf0 = [0 - 31], bf1 = [32 - 63], bf2 = [64 - 95], bf3 = [96 - 127], etc. @@ -472,6 +479,8 @@ export function bloomFindPossibleInjector(startInjector: LInjector, bloomBit: nu // this injector is a potential match. if ((value & mask) === mask) { return injector; + } else if (flags & InjectFlags.Self || flags & InjectFlags.Host && !sameHostView(injector)) { + return null; } // If the current injector does not have the directive, check the bloom filters for the ancestor @@ -491,6 +500,16 @@ export function bloomFindPossibleInjector(startInjector: LInjector, bloomBit: nu return null; } +/** + * Checks whether the current injector and its parent are in the same host view. + * + * This is necessary to support @Host() decorators. If @Host() is set, we should stop searching once + * the injector and its parent view don't match because it means we'd cross the view boundary. + */ +function sameHostView(injector: LInjector): boolean { + return !!injector.parent && injector.parent.node.view === injector.node.view; +} + export class ReadFromInjectorFn { constructor(readonly read: (injector: LInjector, node: LNode, directiveIndex?: number) => T) {} } diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index d2f31dfa1f..4aff404156 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -584,6 +584,9 @@ { "name": "resolveRendererType2" }, + { + "name": "sameHostView" + }, { "name": "saveNameToExportMap" }, diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index 520668381c..ff472f96c3 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 {ChangeDetectorRef, ElementRef, InjectFlags, TemplateRef, ViewContainerRef, defineInjectable} from '@angular/core'; +import {ChangeDetectorRef, ElementRef, Host, InjectFlags, Optional, Self, SkipSelf, TemplateRef, ViewContainerRef, defineInjectable} from '@angular/core'; import {RenderFlags} from '@angular/core/src/render3/interfaces/definition'; import {defineComponent} from '../../src/render3/definition'; @@ -129,8 +129,8 @@ describe('di', () => { } /**
*/ - const App = createComponent('app', function(ctx: any, cm: boolean) { - if (cm) { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { elementStart(0, 'div', ['dirA', '', 'dirB', '']); elementEnd(); } @@ -153,8 +153,8 @@ describe('di', () => { } /** */ - const App = createComponent('app', function(ctx: any, cm: boolean) { - if (cm) { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { elementStart(0, 'comp', ['dirB', '']); elementEnd(); } @@ -180,8 +180,8 @@ describe('di', () => { *
* % } */ - const App = createComponent('app', function(ctx: any, cm: boolean) { - if (cm) { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { container(0); } containerRefreshStart(0); @@ -240,8 +240,8 @@ describe('di', () => { } /**
*/ - const App = createComponent('app', function(ctx: any, cm: boolean) { - if (cm) { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { elementStart(0, 'div', ['dirA', '', 'dirB', '', 'dirC', '']); elementEnd(); } @@ -300,8 +300,8 @@ describe('di', () => { } /** */ - const App = createComponent('app', function(ctx: any, cm: boolean) { - if (cm) { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { elementStart(0, 'comp', ['dirA', '', 'dirB', '', 'dirC', '', 'dirD', '']); elementEnd(); } @@ -378,16 +378,16 @@ describe('di', () => { } /**
*/ - const Parent = createComponent('parent', function(ctx: any, cm: boolean) { - if (cm) { + const Parent = createComponent('parent', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { elementStart(0, 'div', ['dirA', '', 'dirB', '']); elementEnd(); } }, [DirA, DirB]); /** */ - const App = createComponent('app', function(ctx: any, cm: boolean) { - if (cm) { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { elementStart(0, 'parent', ['dirB', '']); elementEnd(); } @@ -426,7 +426,7 @@ describe('di', () => { expect(fixture.html).toEqual('MyService'); }); - it('should throw if directive is not found', () => { + it('should throw if directive is not found anywhere', () => { class Dir { constructor(siblingDir: OtherDir) {} @@ -448,8 +448,8 @@ describe('di', () => { } /**
*/ - const App = createComponent('app', function(ctx: any, cm: boolean) { - if (cm) { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { elementStart(0, 'div', ['dir', '']); elementEnd(); } @@ -458,6 +458,44 @@ describe('di', () => { expect(() => new ComponentFixture(App)).toThrowError(/Injector: NOT_FOUND \[OtherDir\]/); }); + it('should throw if directive is not found in ancestor tree', () => { + class Dir { + constructor(siblingDir: OtherDir) {} + + static ngDirectiveDef = defineDirective({ + selectors: [['', 'dir', '']], + type: Dir, + factory: () => new Dir(directiveInject(OtherDir)), + features: [PublicFeature] + }); + } + + class OtherDir { + static ngDirectiveDef = defineDirective({ + selectors: [['', 'other', '']], + type: OtherDir, + factory: () => new OtherDir(), + features: [PublicFeature] + }); + } + + /** + *
+ *
+ */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', ['other', '']); + elementEnd(); + elementStart(1, 'div', ['dir', '']); + elementEnd(); + } + }, [Dir, OtherDir]); + + expect(() => new ComponentFixture(App)).toThrowError(/Injector: NOT_FOUND \[OtherDir\]/); + }); + + it('should throw if directives try to inject each other', () => { class DirA { constructor(dir: DirB) {} @@ -482,8 +520,8 @@ describe('di', () => { } /**
*/ - const App = createComponent('app', function(ctx: any, cm: boolean) { - if (cm) { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { elementStart(0, 'div', ['dirA', '', 'dirB', '']); elementEnd(); } @@ -505,8 +543,8 @@ describe('di', () => { } /**
*/ - const App = createComponent('app', function(ctx: any, cm: boolean) { - if (cm) { + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { elementStart(0, 'div', ['dir', '']); elementEnd(); } @@ -515,6 +553,217 @@ describe('di', () => { expect(() => new ComponentFixture(App)).toThrowError(/Cannot instantiate cyclic dependency!/); }); + describe('flags', () => { + + class DirB { + value: string; + + static ngDirectiveDef = defineDirective({ + type: DirB, + selectors: [['', 'dirB', '']], + factory: () => new DirB(), + inputs: {value: 'dirB'}, + features: [PublicFeature] + }); + } + + it('should not throw if dependency is @Optional', () => { + 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) { + elementStart(0, 'div', ['dirA', '']); + elementEnd(); + } + }, [DirA, DirB]); + + expect(() => { + const fixture = new ComponentFixture(App); + 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) { + elementStart(0, 'div', ['dirB', '']); + elementEnd(); + elementStart(1, 'div', ['dirA', '']); + elementEnd(); + } + }, [DirA, DirB]); + + expect(() => { + const fixture = new ComponentFixture(App); + expect(dirA !.dirB).toEqual(null); + }).not.toThrow(); + }); + + it('should skip the current node with @SkipSelf', () => { + let dirA: DirA; + + class DirA { + constructor(@SkipSelf() public dirB: DirB) {} + + static ngDirectiveDef = defineDirective({ + type: DirA, + selectors: [['', 'dirA', '']], + factory: () => dirA = new DirA(directiveInject(DirB, InjectFlags.SkipSelf)) + }); + } + + /**
*/ + const Comp = createComponent('comp', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', ['dirA', '', 'dirB', 'self']); + elementEnd(); + } + }, [DirA, DirB]); + + /* */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'comp', ['dirB', 'parent']); + elementEnd(); + } + }, [Comp, DirB]); + + const fixture = new ComponentFixture(App); + expect(dirA !.dirB.value).toEqual('parent'); + }); + + it('should check only the current node with @Self', () => { + let dirA: DirA; + + class DirA { + constructor(@Self() public dirB: DirB) {} + + static ngDirectiveDef = defineDirective({ + type: DirA, + selectors: [['', 'dirA', '']], + factory: () => dirA = new DirA(directiveInject(DirB, InjectFlags.Self)) + }); + } + + /** + *
+ *
+ *
+ */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', ['dirB', '']); + elementStart(1, 'div', ['dirA', '']); + elementEnd(); + elementEnd(); + } + }, [DirA, DirB]); + + expect(() => { + const fixture = new ComponentFixture(App); + }).toThrowError(/Injector: NOT_FOUND \[DirB\]/); + }); + + it('should check only the current node with @Self even with false positive', () => { + let dirA: DirA; + + class DirA { + constructor(@Self() public dirB: DirB) {} + + static ngDirectiveDef = defineDirective({ + type: DirA, + selectors: [['', 'dirA', '']], + factory: () => dirA = new DirA(directiveInject(DirB, InjectFlags.Self)) + }); + } + + const DirC = createDirective('dirC'); + + /** + *
+ *
+ *
+ */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', ['dirB', '']); + elementStart(1, 'div', ['dirA', '', 'dirC', '']); + elementEnd(); + elementEnd(); + } + }, [DirA, DirB, DirC]); + + expect(() => { + (DirA as any)['__NG_ELEMENT_ID__'] = 1; + (DirC as any)['__NG_ELEMENT_ID__'] = 257; + const fixture = new ComponentFixture(App); + }).toThrowError(/Injector: NOT_FOUND \[DirB\]/); + }); + + it('should not pass component boundary with @Host', () => { + let dirA: DirA; + + class DirA { + constructor(@Host() public dirB: DirB) {} + + static ngDirectiveDef = defineDirective({ + type: DirA, + selectors: [['', 'dirA', '']], + factory: () => dirA = new DirA(directiveInject(DirB, InjectFlags.Host)) + }); + } + + /**
*/ + const Comp = createComponent('comp', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', ['dirA', '']); + elementEnd(); + } + }, [DirA, DirB]); + + /* */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'comp', ['dirB', '']); + elementEnd(); + } + }, [Comp, DirB]); + + expect(() => { + const fixture = new ComponentFixture(App); + }).toThrowError(/Injector: NOT_FOUND \[DirB\]/); + + }); + + }); + }); describe('ElementRef', () => { @@ -1025,16 +1274,16 @@ describe('di', () => { bloomAdd(di, { __NG_ELEMENT_ID__: 223 } as any); bloomAdd(di, { __NG_ELEMENT_ID__: 255 } as any); - expect(bloomFindPossibleInjector(di, 0)).toEqual(di); - expect(bloomFindPossibleInjector(di, 1)).toEqual(null); - expect(bloomFindPossibleInjector(di, 32)).toEqual(di); - expect(bloomFindPossibleInjector(di, 64)).toEqual(di); - expect(bloomFindPossibleInjector(di, 96)).toEqual(di); - expect(bloomFindPossibleInjector(di, 127)).toEqual(di); - expect(bloomFindPossibleInjector(di, 161)).toEqual(di); - expect(bloomFindPossibleInjector(di, 188)).toEqual(di); - expect(bloomFindPossibleInjector(di, 223)).toEqual(di); - expect(bloomFindPossibleInjector(di, 255)).toEqual(di); + expect(bloomFindPossibleInjector(di, 0, InjectFlags.Default)).toEqual(di); + expect(bloomFindPossibleInjector(di, 1, InjectFlags.Default)).toEqual(null); + expect(bloomFindPossibleInjector(di, 32, InjectFlags.Default)).toEqual(di); + expect(bloomFindPossibleInjector(di, 64, InjectFlags.Default)).toEqual(di); + expect(bloomFindPossibleInjector(di, 96, InjectFlags.Default)).toEqual(di); + expect(bloomFindPossibleInjector(di, 127, InjectFlags.Default)).toEqual(di); + expect(bloomFindPossibleInjector(di, 161, InjectFlags.Default)).toEqual(di); + expect(bloomFindPossibleInjector(di, 188, InjectFlags.Default)).toEqual(di); + expect(bloomFindPossibleInjector(di, 223, InjectFlags.Default)).toEqual(di); + expect(bloomFindPossibleInjector(di, 255, InjectFlags.Default)).toEqual(di); }); });