Skip to content

Commit 9c1735b

Browse files
alxhubthePunderWoman
alxhub
authored and
thePunderWoman
committed
fix(core): set correct context for inject() for component ctors (#45991)
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
1 parent e6f435a commit 9c1735b

7 files changed

Lines changed: 335 additions & 4 deletions

File tree

‎goldens/size-tracking/integration-payloads.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"main": "TODO(i): temporarily increase the payload size limit from 17597 - this needs to be investigate further what caused the increase.",
1313
"main": "Likely there is a missing PURE annotation https://github.com/angular/angular/pull/43344",
1414
"main": "Tracking issue: https://github.com/angular/angular/issues/43568",
15-
"main": 20826,
15+
"main": 23891,
1616
"polyfills": 33848
1717
}
1818
},
@@ -68,4 +68,4 @@
6868
"bundle": 1214857
6969
}
7070
}
71-
}
71+
}

‎packages/core/src/render3/instructions/shared.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {getFirstLContainer, getLViewParent, getNextLContainer} from '../util/vie
4646
import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, isCreationMode, resetPreOrderHookFlags, unwrapLView, updateTransplantedViewCount, viewAttachedToChangeDetector} from '../util/view_utils';
4747

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

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

‎packages/core/test/acceptance/di_spec.ts‎

Lines changed: 153 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*/
88

99
import {CommonModule} from '@angular/common';
10-
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';
10+
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';
1111
import {ViewRef as ViewRefInternal} from '@angular/core/src/render3/view_ref';
1212
import {TestBed} from '@angular/core/testing';
1313
import {By} from '@angular/platform-browser';
@@ -3177,6 +3177,158 @@ describe('di', () => {
31773177
});
31783178
});
31793179

3180+
describe('inject()', () => {
3181+
it('should work in a directive constructor', () => {
3182+
const TOKEN = new InjectionToken<string>('TOKEN');
3183+
3184+
@Component({
3185+
standalone: true,
3186+
selector: 'test-cmp',
3187+
template: '{{value}}',
3188+
providers: [{provide: TOKEN, useValue: 'injected value'}],
3189+
})
3190+
class TestCmp {
3191+
value: string;
3192+
constructor() {
3193+
this.value = inject(TOKEN);
3194+
}
3195+
}
3196+
3197+
const fixture = TestBed.createComponent(TestCmp);
3198+
fixture.detectChanges();
3199+
expect(fixture.nativeElement.innerHTML).toEqual('injected value');
3200+
});
3201+
3202+
it('should work in a service constructor when the service is provided on a directive', () => {
3203+
const TOKEN = new InjectionToken<string>('TOKEN');
3204+
3205+
@Injectable()
3206+
class Service {
3207+
value: string;
3208+
constructor() {
3209+
this.value = inject(TOKEN);
3210+
}
3211+
}
3212+
3213+
@Component({
3214+
standalone: true,
3215+
selector: 'test-cmp',
3216+
template: '{{service.value}}',
3217+
providers: [Service, {provide: TOKEN, useValue: 'injected value'}],
3218+
})
3219+
class TestCmp {
3220+
constructor(readonly service: Service) {}
3221+
}
3222+
3223+
const fixture = TestBed.createComponent(TestCmp);
3224+
fixture.detectChanges();
3225+
expect(fixture.nativeElement.innerHTML).toEqual('injected value');
3226+
});
3227+
3228+
3229+
it('should be able to inject special tokens like ChangeDetectorRef', () => {
3230+
const TOKEN = new InjectionToken<string>('TOKEN');
3231+
3232+
@Component({
3233+
standalone: true,
3234+
selector: 'test-cmp',
3235+
template: '{{value}}',
3236+
})
3237+
class TestCmp {
3238+
cdr = inject(ChangeDetectorRef);
3239+
value = 'before';
3240+
}
3241+
3242+
const fixture = TestBed.createComponent(TestCmp);
3243+
fixture.componentInstance.value = 'after';
3244+
fixture.componentInstance.cdr.detectChanges();
3245+
expect(fixture.nativeElement.innerHTML).toEqual('after');
3246+
});
3247+
3248+
it('should work in a service constructor', () => {
3249+
const TOKEN = new InjectionToken<string>('TOKEN', {
3250+
providedIn: 'root',
3251+
factory: () => 'injected value',
3252+
});
3253+
3254+
@Injectable({providedIn: 'root'})
3255+
class Service {
3256+
value: string;
3257+
constructor() {
3258+
this.value = inject(TOKEN);
3259+
}
3260+
}
3261+
3262+
const service = TestBed.inject(Service);
3263+
expect(service.value).toEqual('injected value');
3264+
});
3265+
3266+
it('should work in a useFactory definition for a service', () => {
3267+
const TOKEN = new InjectionToken<string>('TOKEN', {
3268+
providedIn: 'root',
3269+
factory: () => 'injected value',
3270+
});
3271+
3272+
@Injectable({
3273+
providedIn: 'root',
3274+
useFactory: () => new Service(inject(TOKEN)),
3275+
})
3276+
class Service {
3277+
constructor(readonly value: string) {}
3278+
}
3279+
3280+
expect(TestBed.inject(Service).value).toEqual('injected value');
3281+
});
3282+
3283+
it('should work for field injection', () => {
3284+
const TOKEN = new InjectionToken<string>('TOKEN', {
3285+
providedIn: 'root',
3286+
factory: () => 'injected value',
3287+
});
3288+
3289+
@Injectable({providedIn: 'root'})
3290+
class Service {
3291+
value = inject(TOKEN);
3292+
}
3293+
3294+
const service = TestBed.inject(Service);
3295+
expect(service.value).toEqual('injected value');
3296+
});
3297+
3298+
it('should not give non-node services access to the node context', () => {
3299+
const TOKEN = new InjectionToken<string>('TOKEN');
3300+
3301+
@Injectable({providedIn: 'root'})
3302+
class Service {
3303+
value: string;
3304+
constructor() {
3305+
this.value = inject(TOKEN, InjectFlags.Optional) ?? 'default value';
3306+
}
3307+
}
3308+
3309+
@Component({
3310+
standalone: true,
3311+
selector: 'test-cmp',
3312+
template: '{{service.value}}',
3313+
providers: [{provide: TOKEN, useValue: 'injected value'}],
3314+
})
3315+
class TestCmp {
3316+
service: Service;
3317+
constructor() {
3318+
// `Service` is injected starting from the component context, where `inject` is
3319+
// `ɵɵdirectiveInject` under the hood. However, this should reach the root injector which
3320+
// should _not_ use `ɵɵdirectiveInject` to inject dependencies of `Service`, so `TOKEN`
3321+
// should not be visible to `Service`.
3322+
this.service = inject(Service);
3323+
}
3324+
}
3325+
3326+
const fixture = TestBed.createComponent(TestCmp);
3327+
fixture.detectChanges();
3328+
expect(fixture.nativeElement.innerHTML).toEqual('default value');
3329+
});
3330+
});
3331+
31803332
it('should be able to use Host in `useFactory` dependency config', () => {
31813333
// Scenario:
31823334
// ---------

‎packages/core/test/bundling/animations/bundle.golden_symbols.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,9 @@
854854
{
855855
"name": "getNullInjector"
856856
},
857+
{
858+
"name": "getOrCreateInjectable"
859+
},
857860
{
858861
"name": "getOrCreateNodeInjectorForNode"
859862
},
@@ -1391,6 +1394,9 @@
13911394
{
13921395
"name": "ɵɵdefineNgModule"
13931396
},
1397+
{
1398+
"name": "ɵɵdirectiveInject"
1399+
},
13941400
{
13951401
"name": "ɵɵelement"
13961402
},

0 commit comments

Comments
 (0)