Skip to content

Commit

Permalink
fix(core): set correct context for inject() for component ctors (#45991)
Browse files Browse the repository at this point in the history
The `inject()` function was introduced with Ivy to support imperative
injection in factory/constructor contexts, such as directive or service
constructors as well as factory functions defined in `@Injectable` or
`InjectionToken`. However, `inject()` in a component/directive constructor
did not work due to a flaw in the logic for creating the internal factory
for components/directives.

The original intention of this logic was to keep `ɵɵdirectiveInject` tree-
shakable for applications which don't use any component-level DI. However,
this breaks the `inject()` functionality for component/directive
constructors.

This commit fixes that issue and adds tests for all the various cases in
which `inject()` should function. As a result `ɵɵdirectiveInject` is no
longer tree-shakable, but that's totally acceptable as any application that
uses `*ngIf` or `*ngFor` already contains this function. It's possible to
change how `inject()` works to restore this tree-shakability if needed.

PR Close #45991
  • Loading branch information
alxhub authored and thePunderWoman committed May 13, 2022
1 parent 45fedb5 commit 3f7ecec
Show file tree
Hide file tree
Showing 7 changed files with 335 additions and 4 deletions.
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
Expand Up @@ -12,7 +12,7 @@
"main": "TODO(i): temporarily increase the payload size limit from 17597 - this needs to be investigate further what caused the increase.",
"main": "Likely there is a missing PURE annotation https://github.com/angular/angular/pull/43344",
"main": "Tracking issue: https://github.com/angular/angular/issues/43568",
"main": 20826,
"main": 23891,
"polyfills": 33848
}
},
Expand Down Expand Up @@ -68,4 +68,4 @@
"bundle": 1214857
}
}
}
}
7 changes: 6 additions & 1 deletion packages/core/src/render3/instructions/shared.ts
Expand Up @@ -46,6 +46,7 @@ import {getFirstLContainer, getLViewParent, getNextLContainer} from '../util/vie
import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, isCreationMode, resetPreOrderHookFlags, unwrapLView, updateTransplantedViewCount, viewAttachedToChangeDetector} from '../util/view_utils';

import {selectIndexInternal} from './advance';
import {ɵɵdirectiveInject} from './di';
import {attachLContainerDebug, attachLViewDebug, cloneToLViewFromTViewBlueprint, cloneToTViewData, LCleanup, LViewBlueprint, MatchesArray, TCleanup, TNodeDebug, TNodeInitialInputs, TNodeLocalNames, TViewComponents, TViewConstructor} from './lview_debug';

let shouldThrowErrorOnUnknownProperty = false;
Expand Down Expand Up @@ -1532,7 +1533,11 @@ function configureViewWithDirective<T>(
tView.data[directiveIndex] = def;
const directiveFactory =
def.factory || ((def as {factory: Function}).factory = getFactoryDef(def.type, true));
const nodeInjectorFactory = new NodeInjectorFactory(directiveFactory, isComponentDef(def), null);
// Even though `directiveFactory` will already be using `ɵɵdirectiveInject` in its generated code,
// we also want to support `inject()` directly from the directive constructor context so we set
// `ɵɵdirectiveInject` as the inject implementation here too.
const nodeInjectorFactory =
new NodeInjectorFactory(directiveFactory, isComponentDef(def), ɵɵdirectiveInject);
tView.blueprint[directiveIndex] = nodeInjectorFactory;
lView[directiveIndex] = nodeInjectorFactory;

Expand Down
154 changes: 153 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, createEnvironmentInjector, Directive, ElementRef, ENVIRONMENT_INITIALIZER, EventEmitter, forwardRef, Host, HostBinding, ImportedNgModuleProviders, importProvidersFrom, ImportProvidersSource, Inject, Injectable, InjectFlags, InjectionToken, INJECTOR, Injector, Input, LOCALE_ID, ModuleWithProviders, NgModule, NgZone, Optional, Output, Pipe, PipeTransform, Provider, Self, SkipSelf, TemplateRef, Type, ViewChild, ViewContainerRef, ViewRef, ɵcreateInjector as createInjector, ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID, ɵINJECTOR_SCOPE} from '@angular/core';
import {Attribute, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, createEnvironmentInjector, Directive, ElementRef, ENVIRONMENT_INITIALIZER, EventEmitter, forwardRef, Host, HostBinding, ImportedNgModuleProviders, importProvidersFrom, ImportProvidersSource, inject, Inject, Injectable, InjectFlags, InjectionToken, INJECTOR, Injector, Input, LOCALE_ID, ModuleWithProviders, NgModule, NgZone, Optional, Output, Pipe, PipeTransform, Provider, Self, SkipSelf, TemplateRef, Type, ViewChild, ViewContainerRef, ViewRef, ɵcreateInjector as createInjector, ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID, ɵINJECTOR_SCOPE} from '@angular/core';
import {ViewRef as ViewRefInternal} from '@angular/core/src/render3/view_ref';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
Expand Down Expand Up @@ -3177,6 +3177,158 @@ describe('di', () => {
});
});

describe('inject()', () => {
it('should work in a directive constructor', () => {
const TOKEN = new InjectionToken<string>('TOKEN');

@Component({
standalone: true,
selector: 'test-cmp',
template: '{{value}}',
providers: [{provide: TOKEN, useValue: 'injected value'}],
})
class TestCmp {
value: string;
constructor() {
this.value = inject(TOKEN);
}
}

const fixture = TestBed.createComponent(TestCmp);
fixture.detectChanges();
expect(fixture.nativeElement.innerHTML).toEqual('injected value');
});

it('should work in a service constructor when the service is provided on a directive', () => {
const TOKEN = new InjectionToken<string>('TOKEN');

@Injectable()
class Service {
value: string;
constructor() {
this.value = inject(TOKEN);
}
}

@Component({
standalone: true,
selector: 'test-cmp',
template: '{{service.value}}',
providers: [Service, {provide: TOKEN, useValue: 'injected value'}],
})
class TestCmp {
constructor(readonly service: Service) {}
}

const fixture = TestBed.createComponent(TestCmp);
fixture.detectChanges();
expect(fixture.nativeElement.innerHTML).toEqual('injected value');
});


it('should be able to inject special tokens like ChangeDetectorRef', () => {
const TOKEN = new InjectionToken<string>('TOKEN');

@Component({
standalone: true,
selector: 'test-cmp',
template: '{{value}}',
})
class TestCmp {
cdr = inject(ChangeDetectorRef);
value = 'before';
}

const fixture = TestBed.createComponent(TestCmp);
fixture.componentInstance.value = 'after';
fixture.componentInstance.cdr.detectChanges();
expect(fixture.nativeElement.innerHTML).toEqual('after');
});

it('should work in a service constructor', () => {
const TOKEN = new InjectionToken<string>('TOKEN', {
providedIn: 'root',
factory: () => 'injected value',
});

@Injectable({providedIn: 'root'})
class Service {
value: string;
constructor() {
this.value = inject(TOKEN);
}
}

const service = TestBed.inject(Service);
expect(service.value).toEqual('injected value');
});

it('should work in a useFactory definition for a service', () => {
const TOKEN = new InjectionToken<string>('TOKEN', {
providedIn: 'root',
factory: () => 'injected value',
});

@Injectable({
providedIn: 'root',
useFactory: () => new Service(inject(TOKEN)),
})
class Service {
constructor(readonly value: string) {}
}

expect(TestBed.inject(Service).value).toEqual('injected value');
});

it('should work for field injection', () => {
const TOKEN = new InjectionToken<string>('TOKEN', {
providedIn: 'root',
factory: () => 'injected value',
});

@Injectable({providedIn: 'root'})
class Service {
value = inject(TOKEN);
}

const service = TestBed.inject(Service);
expect(service.value).toEqual('injected value');
});

it('should not give non-node services access to the node context', () => {
const TOKEN = new InjectionToken<string>('TOKEN');

@Injectable({providedIn: 'root'})
class Service {
value: string;
constructor() {
this.value = inject(TOKEN, InjectFlags.Optional) ?? 'default value';
}
}

@Component({
standalone: true,
selector: 'test-cmp',
template: '{{service.value}}',
providers: [{provide: TOKEN, useValue: 'injected value'}],
})
class TestCmp {
service: Service;
constructor() {
// `Service` is injected starting from the component context, where `inject` is
// `ɵɵdirectiveInject` under the hood. However, this should reach the root injector which
// should _not_ use `ɵɵdirectiveInject` to inject dependencies of `Service`, so `TOKEN`
// should not be visible to `Service`.
this.service = inject(Service);
}
}

const fixture = TestBed.createComponent(TestCmp);
fixture.detectChanges();
expect(fixture.nativeElement.innerHTML).toEqual('default value');
});
});

it('should be able to use Host in `useFactory` dependency config', () => {
// Scenario:
// ---------
Expand Down
Expand Up @@ -854,6 +854,9 @@
{
"name": "getNullInjector"
},
{
"name": "getOrCreateInjectable"
},
{
"name": "getOrCreateNodeInjectorForNode"
},
Expand Down Expand Up @@ -1391,6 +1394,9 @@
{
"name": "ɵɵdefineNgModule"
},
{
"name": "ɵɵdirectiveInject"
},
{
"name": "ɵɵelement"
},
Expand Down

0 comments on commit 3f7ecec

Please sign in to comment.