Skip to content

Commit

Permalink
fix(core): use correct injector when resolving DI tokens from within …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
JoostK authored and alxhub committed Jul 20, 2021
1 parent 778edfc commit a6db152
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 2 deletions.
5 changes: 4 additions & 1 deletion packages/core/src/di/r3_injector.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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);
}
}
Expand Down
89 changes: 88 additions & 1 deletion packages/core/test/acceptance/di_spec.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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]'})
Expand Down
Expand Up @@ -92,6 +92,9 @@
{
"name": "injectArgs"
},
{
"name": "injectInjectorOnly"
},
{
"name": "injectableDefOrInjectorDefFactory"
},
Expand All @@ -110,6 +113,9 @@
{
"name": "setCurrentInjector"
},
{
"name": "setInjectImplementation"
},
{
"name": "stringify"
},
Expand Down

0 comments on commit a6db152

Please sign in to comment.