From 929334b0bf64c225c776d0e4643e9078779ec418 Mon Sep 17 00:00:00 2001 From: Marc Laval Date: Thu, 3 Jan 2019 14:43:06 +0100 Subject: [PATCH] fix(ivy): should not throw when getting VCRef.parentInjector on the root view (#27909) PR Close #27909 --- packages/core/src/render3/di.ts | 145 +++++++++--------- .../src/render3/view_engine_compatibility.ts | 2 +- .../bundling/todo/bundle.golden_symbols.json | 6 - .../linker/view_injector_integration_spec.ts | 75 +++++---- .../test/render3/view_container_ref_spec.ts | 17 ++ 5 files changed, 129 insertions(+), 116 deletions(-) diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 2ea1df8f46..63ad94ed1c 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -208,7 +208,7 @@ export function getParentInjectorLocation(tNode: TNode, view: LView): RelativeIn let viewOffset = 1; while (hostTNode && hostTNode.injectorIndex === -1) { view = view[DECLARATION_VIEW] !; - hostTNode = view[HOST_NODE] !; + hostTNode = view ? view[HOST_NODE] : null; viewOffset++; } @@ -292,82 +292,86 @@ export function injectAttributeImpl(tNode: TNode, attrNameToInject: string): str * @returns the value from the injector, `null` when not found, or `notFoundValue` if provided */ export function getOrCreateInjectable( - tNode: TElementNode | TContainerNode | TElementContainerNode, lView: LView, + tNode: TElementNode | TContainerNode | TElementContainerNode | null, lView: LView, token: Type| InjectionToken, flags: InjectFlags = InjectFlags.Default, notFoundValue?: any): T|null { - const bloomHash = bloomHashBitOrFactory(token); - // If the ID stored here is a function, this is a special object like ElementRef or TemplateRef - // so just call the factory function to create it. - if (typeof bloomHash === 'function') { - const savePreviousOrParentTNode = getPreviousOrParentTNode(); - const saveLView = getLView(); - setTNodeAndViewData(tNode, lView); - try { - const value = bloomHash(); - if (value == null && !(flags & InjectFlags.Optional)) { - throw new Error(`No provider for ${stringify(token)}!`); - } else { - return value; + if (tNode) { + const bloomHash = bloomHashBitOrFactory(token); + // If the ID stored here is a function, this is a special object like ElementRef or TemplateRef + // so just call the factory function to create it. + if (typeof bloomHash === 'function') { + const savePreviousOrParentTNode = getPreviousOrParentTNode(); + const saveLView = getLView(); + setTNodeAndViewData(tNode, lView); + try { + const value = bloomHash(); + if (value == null && !(flags & InjectFlags.Optional)) { + throw new Error(`No provider for ${stringify(token)}!`); + } else { + return value; + } + } finally { + setTNodeAndViewData(savePreviousOrParentTNode, saveLView); } - } finally { - setTNodeAndViewData(savePreviousOrParentTNode, saveLView); - } - } else if (typeof bloomHash == 'number') { - // If the token has a bloom hash, then it is a token which could be in NodeInjector. + } else if (typeof bloomHash == 'number') { + // If the token has a bloom hash, then it is a token which could be in NodeInjector. - // A reference to the previous injector TView that was found while climbing the element injector - // tree. This is used to know if viewProviders can be accessed on the current injector. - let previousTView: TView|null = null; - let injectorIndex = getInjectorIndex(tNode, lView); - let parentLocation: RelativeInjectorLocation = NO_PARENT_INJECTOR; - let hostTElementNode: TNode|null = - flags & InjectFlags.Host ? findComponentView(lView)[HOST_NODE] : null; + // A reference to the previous injector TView that was found while climbing the element + // injector tree. This is used to know if viewProviders can be accessed on the current + // injector. + let previousTView: TView|null = null; + let injectorIndex = getInjectorIndex(tNode, lView); + let parentLocation: RelativeInjectorLocation = NO_PARENT_INJECTOR; + let hostTElementNode: TNode|null = + flags & InjectFlags.Host ? findComponentView(lView)[HOST_NODE] : null; - // If we should skip this injector, or if there is no injector on this node, start by searching - // the parent injector. - if (injectorIndex === -1 || flags & InjectFlags.SkipSelf) { - parentLocation = injectorIndex === -1 ? getParentInjectorLocation(tNode, lView) : - lView[injectorIndex + PARENT_INJECTOR]; + // If we should skip this injector, or if there is no injector on this node, start by + // searching + // the parent injector. + if (injectorIndex === -1 || flags & InjectFlags.SkipSelf) { + parentLocation = injectorIndex === -1 ? getParentInjectorLocation(tNode, lView) : + lView[injectorIndex + PARENT_INJECTOR]; - if (!shouldSearchParent(flags, false)) { - injectorIndex = -1; - } else { - previousTView = lView[TVIEW]; - injectorIndex = getParentInjectorIndex(parentLocation); - lView = getParentInjectorView(parentLocation, lView); - } - } - - // Traverse up the injector tree until we find a potential match or until we know there - // *isn't* a match. - while (injectorIndex !== -1) { - parentLocation = lView[injectorIndex + PARENT_INJECTOR]; - - // Check the current injector. If it matches, see if it contains token. - const tView = lView[TVIEW]; - if (bloomHasToken(bloomHash, injectorIndex, tView.data)) { - // At this point, we have an injector which *may* contain the token, so we step through - // the providers and directives associated with the injector's corresponding node to get - // the instance. - const instance: T|null = searchTokensOnInjector( - injectorIndex, lView, token, previousTView, flags, hostTElementNode); - if (instance !== NOT_FOUND) { - return instance; + if (!shouldSearchParent(flags, false)) { + injectorIndex = -1; + } else { + previousTView = lView[TVIEW]; + injectorIndex = getParentInjectorIndex(parentLocation); + lView = getParentInjectorView(parentLocation, lView); } } - if (shouldSearchParent( - flags, lView[TVIEW].data[injectorIndex + TNODE] === hostTElementNode) && - bloomHasToken(bloomHash, injectorIndex, lView)) { - // The def wasn't found anywhere on this node, so it was a false positive. - // Traverse up the tree and continue searching. - previousTView = tView; - injectorIndex = getParentInjectorIndex(parentLocation); - lView = getParentInjectorView(parentLocation, lView); - } else { - // If we should not search parent OR If the ancestor bloom filter value does not have the - // bit corresponding to the directive we can give up on traversing up to find the specific - // injector. - injectorIndex = -1; + + // Traverse up the injector tree until we find a potential match or until we know there + // *isn't* a match. + while (injectorIndex !== -1) { + parentLocation = lView[injectorIndex + PARENT_INJECTOR]; + + // Check the current injector. If it matches, see if it contains token. + const tView = lView[TVIEW]; + if (bloomHasToken(bloomHash, injectorIndex, tView.data)) { + // At this point, we have an injector which *may* contain the token, so we step through + // the providers and directives associated with the injector's corresponding node to get + // the instance. + const instance: T|null = searchTokensOnInjector( + injectorIndex, lView, token, previousTView, flags, hostTElementNode); + if (instance !== NOT_FOUND) { + return instance; + } + } + if (shouldSearchParent( + flags, lView[TVIEW].data[injectorIndex + TNODE] === hostTElementNode) && + bloomHasToken(bloomHash, injectorIndex, lView)) { + // The def wasn't found anywhere on this node, so it was a false positive. + // Traverse up the tree and continue searching. + previousTView = tView; + injectorIndex = getParentInjectorIndex(parentLocation); + lView = getParentInjectorView(parentLocation, lView); + } else { + // If we should not search parent OR If the ancestor bloom filter value does not have the + // bit corresponding to the directive we can give up on traversing up to find the specific + // injector. + injectorIndex = -1; + } } } } @@ -570,7 +574,8 @@ export function injectInjector() { export class NodeInjector implements Injector { constructor( - private _tNode: TElementNode|TContainerNode|TElementContainerNode, private _lView: LView) {} + private _tNode: TElementNode|TContainerNode|TElementContainerNode|null, + private _lView: LView) {} get(token: any, notFoundValue?: any): any { return getOrCreateInjectable(this._tNode, this._lView, token, undefined, notFoundValue); diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts index 0f924ba8c9..038e11fbed 100644 --- a/packages/core/src/render3/view_engine_compatibility.ts +++ b/packages/core/src/render3/view_engine_compatibility.ts @@ -190,7 +190,7 @@ export function createContainerRef( const parentTNode = getParentInjectorTNode(parentLocation, this._hostView, this._hostTNode); return !hasParentInjector(parentLocation) || parentTNode == null ? - new NullInjector() : + new NodeInjector(null, this._hostView) : new NodeInjector(parentTNode, parentView); } diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 86140e2e12..951ea1bfce 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -152,9 +152,6 @@ { "name": "NodeInjectorFactory" }, - { - "name": "NullInjector" - }, { "name": "ObjectUnsubscribedErrorImpl" }, @@ -269,9 +266,6 @@ { "name": "_DuplicateMap" }, - { - "name": "_THROW_IF_NOT_FOUND" - }, { "name": "__extends" }, diff --git a/packages/core/test/linker/view_injector_integration_spec.ts b/packages/core/test/linker/view_injector_integration_spec.ts index 1202b0c522..9f89b9854f 100644 --- a/packages/core/test/linker/view_injector_integration_spec.ts +++ b/packages/core/test/linker/view_injector_integration_spec.ts @@ -758,25 +758,23 @@ class TestComp { .toThrowError('NodeInjector: NOT_FOUND [SimpleDirective]'); }); - fixmeIvy('FW-638: Exception thrown when getting VCRef.parentInjector on the root view') - .it('should allow to use the NgModule injector from a root ViewContainerRef.parentInjector', - () => { - @Component({template: ''}) - class MyComp { - constructor(public vc: ViewContainerRef) {} - } + it('should allow to use the NgModule injector from a root ViewContainerRef.parentInjector', + () => { + @Component({template: ''}) + class MyComp { + constructor(public vc: ViewContainerRef) {} + } - const compFixture = - TestBed - .configureTestingModule({ - declarations: [MyComp], - providers: [{provide: 'someToken', useValue: 'someValue'}] - }) - .createComponent(MyComp); + const compFixture = TestBed + .configureTestingModule({ + declarations: [MyComp], + providers: [{provide: 'someToken', useValue: 'someValue'}] + }) + .createComponent(MyComp); - expect(compFixture.componentInstance.vc.parentInjector.get('someToken')) - .toBe('someValue'); - }); + expect(compFixture.componentInstance.vc.parentInjector.get('someToken')) + .toBe('someValue'); + }); }); describe('static attributes', () => { @@ -901,31 +899,30 @@ class TestComp { .toBe(el.children[0].nativeElement); }); - fixmeIvy('FW-638: Exception thrown when getting VCRef.parentInjector on the root view') - .it('should inject ViewContainerRef', () => { - @Component({template: ''}) - class TestComp { - constructor(public vcr: ViewContainerRef) {} - } + it('should inject ViewContainerRef', () => { + @Component({template: ''}) + class TestComp { + constructor(public vcr: ViewContainerRef) {} + } - @NgModule({ - declarations: [TestComp], - entryComponents: [TestComp], - }) - class TestModule { - } + @NgModule({ + declarations: [TestComp], + entryComponents: [TestComp], + }) + class TestModule { + } - const testInjector = { - get: (token: any, notFoundValue: any) => - token === 'someToken' ? 'someNewValue' : notFoundValue - }; + const testInjector = { + get: (token: any, notFoundValue: any) => + token === 'someToken' ? 'someNewValue' : notFoundValue + }; - const compFactory = TestBed.configureTestingModule({imports: [TestModule]}) - .get(ComponentFactoryResolver) - .resolveComponentFactory(TestComp); - const component = compFactory.create(testInjector); - expect(component.instance.vcr.parentInjector.get('someToken')).toBe('someNewValue'); - }); + const compFactory = TestBed.configureTestingModule({imports: [TestModule]}) + .get(ComponentFactoryResolver) + .resolveComponentFactory(TestComp); + const component = compFactory.create(testInjector); + expect(component.instance.vcr.parentInjector.get('someToken')).toBe('someNewValue'); + }); it('should inject TemplateRef', () => { TestBed.configureTestingModule({declarations: [NeedsViewContainerRef, NeedsTemplateRef]}); diff --git a/packages/core/test/render3/view_container_ref_spec.ts b/packages/core/test/render3/view_container_ref_spec.ts index 6efbdcd38c..fe64165925 100644 --- a/packages/core/test/render3/view_container_ref_spec.ts +++ b/packages/core/test/render3/view_container_ref_spec.ts @@ -1926,6 +1926,8 @@ describe('ViewContainerRef', () => { } clear() { this._vcRef.clear(); } + + getVCRefParentInjector() { return this._vcRef.parentInjector; } } // https://stackblitz.com/edit/angular-xxpffd?file=src%2Findex.html @@ -1952,6 +1954,21 @@ describe('ViewContainerRef', () => { expect(fixture.outerHtml).toBe('
'); }); + it('should allow getting the parentInjector of the VCRef which was injected into the root (bootstrapped) component', + () => { + const fixture = new ComponentFixture(AppCmpt, { + injector: { + get: (token: any) => { + if (token === 'foo') return 'bar'; + } + } + }); + expect(fixture.outerHtml).toBe('
'); + + const parentInjector = fixture.component.getVCRefParentInjector(); + expect(parentInjector.get('foo')).toEqual('bar'); + }); + it('should check bindings for components dynamically created by root component', () => { class DynamicCompWithBindings { checkCount = 0;