From a6db152c78bc82ef39e529ab5ea55f810b17fa2e Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 17 Jul 2021 21:26:12 +0200 Subject: [PATCH] fix(core): use correct injector when resolving DI tokens from within a directive provider factory (#42886) When a directive provides a DI token using a factory function and interacting with a standalone injector from within that factory, the standalone injector should not have access to either the directive injector nor the NgModule injector; only the standalone injector should be used. This commit ensures that a standalone injector never reaches into the directive-level injection context while resolving DI tokens. Fixes #42651 PR Close #42886 --- packages/core/src/di/r3_injector.ts | 5 +- packages/core/test/acceptance/di_spec.ts | 89 ++++++++++++++++++- .../injection/bundle.golden_symbols.json | 6 ++ 3 files changed, 98 insertions(+), 2 deletions(-) diff --git a/packages/core/src/di/r3_injector.ts b/packages/core/src/di/r3_injector.ts index 61947af699b7d..cf0de1081abb5 100644 --- a/packages/core/src/di/r3_injector.ts +++ b/packages/core/src/di/r3_injector.ts @@ -17,6 +17,7 @@ import {EMPTY_ARRAY} from '../util/empty'; import {stringify} from '../util/stringify'; import {resolveForwardRef} from './forward_ref'; +import {setInjectImplementation} from './inject_switch'; import {InjectionToken} from './injection_token'; import {Injector} from './injector'; import {catchInjectorError, injectArgs, NG_TEMP_TOKEN_PATH, setCurrentInjector, THROW_IF_NOT_FOUND, USE_VALUE, ɵɵinject} from './injector_compatibility'; @@ -186,6 +187,7 @@ export class R3Injector { this.assertNotDestroyed(); // Set the injection context. const previousInjector = setCurrentInjector(this); + const previousInjectImplementation = setInjectImplementation(undefined); try { // Check for the SkipSelf flag. if (!(flags & InjectFlags.SkipSelf)) { @@ -234,7 +236,8 @@ export class R3Injector { throw e; } } finally { - // Lastly, clean up the state by restoring the previous injector. + // Lastly, restore the previous injection context. + setInjectImplementation(previousInjectImplementation); setCurrentInjector(previousInjector); } } diff --git a/packages/core/test/acceptance/di_spec.ts b/packages/core/test/acceptance/di_spec.ts index 7c1001e48a2a4..91f49cc416f5b 100644 --- a/packages/core/test/acceptance/di_spec.ts +++ b/packages/core/test/acceptance/di_spec.ts @@ -7,7 +7,7 @@ */ import {CommonModule} from '@angular/common'; -import {Attribute, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, ContentChild, Directive, ElementRef, EventEmitter, forwardRef, Host, HostBinding, Inject, Injectable, InjectionToken, INJECTOR, Injector, Input, LOCALE_ID, NgModule, NgZone, Optional, Output, Pipe, PipeTransform, Self, SkipSelf, TemplateRef, ViewChild, ViewContainerRef, ViewRef, ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID} from '@angular/core'; +import {Attribute, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, Directive, ElementRef, EventEmitter, forwardRef, Host, HostBinding, Inject, Injectable, InjectionToken, INJECTOR, Injector, Input, LOCALE_ID, NgModule, NgZone, Optional, Output, Pipe, PipeTransform, Self, SkipSelf, TemplateRef, ViewChild, ViewContainerRef, ViewRef, ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID} from '@angular/core'; import {ɵINJECTOR_SCOPE} from '@angular/core/src/core'; import {ViewRef as ViewRefInternal} from '@angular/core/src/render3/view_ref'; import {TestBed} from '@angular/core/testing'; @@ -599,6 +599,93 @@ describe('di', () => { expect(() => TestBed.createComponent(MyComp)).toThrowError(/No provider for DirectiveB/); }); + it('should not have access to the directive injector in a standalone injector from within a directive-level provider factory', + () => { + // https://github.com/angular/angular/issues/42651 + class TestA { + constructor(public injector: string) {} + } + class TestB { + constructor(public a: TestA) {} + } + + function createTestB() { + // Setup a standalone injector that provides `TestA`, which is resolved from a + // standalone child injector that requests `TestA` as a dependency for `TestB`. + // Although we're inside a directive factory and therefore have access to the + // directive-level injector, `TestA` has to be resolved from the standalone injector. + const parent = Injector.create({ + providers: [{provide: TestA, useFactory: () => new TestA('standalone'), deps: []}], + name: 'TestA', + }); + const child = Injector.create({ + providers: [{provide: TestB, useClass: TestB, deps: [TestA]}], + parent, + name: 'TestB', + }); + return child.get(TestB); + } + + @Component({ + template: '', + providers: [ + {provide: TestA, useFactory: () => new TestA('component'), deps: []}, + {provide: TestB, useFactory: createTestB}, + ], + }) + class MyComp { + constructor(public readonly testB: TestB) {} + } + + TestBed.configureTestingModule({declarations: [MyComp]}); + + const cmp = TestBed.createComponent(MyComp); + expect(cmp.componentInstance.testB).toBeInstanceOf(TestB); + expect(cmp.componentInstance.testB.a.injector).toBe('standalone'); + }); + + it('should not have access to the directive injector in a standalone injector from within a directive-level provider factory', + () => { + class TestA { + constructor(public injector: string) {} + } + class TestB { + constructor(public a: TestA|null) {} + } + + function createTestB() { + // Setup a standalone injector that provides `TestB` with an optional dependency of + // `TestA`. Since `TestA` is not provided by the standalone injector it should resolve + // to null; both the NgModule providers and the component-level providers should not + // be considered. + const injector = Injector.create({ + providers: [{provide: TestB, useClass: TestB, deps: [[TestA, new Optional()]]}], + name: 'TestB', + }); + return injector.get(TestB); + } + + @Component({ + template: '', + providers: [ + {provide: TestA, useFactory: () => new TestA('component'), deps: []}, + {provide: TestB, useFactory: createTestB}, + ], + }) + class MyComp { + constructor(public readonly testB: TestB) {} + } + + TestBed.configureTestingModule({ + declarations: [MyComp], + providers: [{provide: TestA, useFactory: () => new TestA('module'), deps: []}] + }); + + const cmp = TestBed.createComponent(MyComp); + expect(cmp.componentInstance.testB).toBeInstanceOf(TestB); + expect(cmp.componentInstance.testB.a).toBeNull(); + }); + onlyInIvy('Ivy has different error message for circular dependency') .it('should throw if directives try to inject each other', () => { @Directive({selector: '[dirB]'}) diff --git a/packages/core/test/bundling/injection/bundle.golden_symbols.json b/packages/core/test/bundling/injection/bundle.golden_symbols.json index 2333422a61b33..4ebbb42bb3bc0 100644 --- a/packages/core/test/bundling/injection/bundle.golden_symbols.json +++ b/packages/core/test/bundling/injection/bundle.golden_symbols.json @@ -92,6 +92,9 @@ { "name": "injectArgs" }, + { + "name": "injectInjectorOnly" + }, { "name": "injectableDefOrInjectorDefFactory" }, @@ -110,6 +113,9 @@ { "name": "setCurrentInjector" }, + { + "name": "setInjectImplementation" + }, { "name": "stringify" },