Skip to content

fix(core): set correct context for inject() for component ctors #45991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
alxhub wants to merge 1 commit into from
Closed

fix(core): set correct context for inject() for component ctors #45991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions goldens/size-tracking/integration-payloads.json
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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
View file
Open in desktop
Original file line number Diff line number Diff line change
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);

@splincode splincode May 18, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alxhub Hello, thank you for this PR! Really great job!

I want to say you about, is this working with custom decorator?

const Autowired = (target: any, memberName: string) => {
  Object.defineProperty(target, memberName, {
    set: (newValue: any) => {
      target[memberName] = newValue;
    },
    get: () => inject(ChangeDetectorRef)
  });
};

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

@JoostK JoostK May 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that particular code wouldn't work as inject is not called during construction, but only in the property's getter. The injection context won't be available at that point in time.

@splincode splincode May 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What code is then valid for writing your own decorator? Tell me please

@JoostK JoostK May 18, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Property decorators don't get to add logic to run during construction, so you can't do that. There's probably ways to get things to work with a supporting class decorator, but I wouldn't recommend that.

Really just use

readonly cdr = inject(ChangeDetectorRef);

which gets you type-safe code without size nor runtime overhead and further magic (you have an additional problem to solve w.r.t. the token to inject, which in your example is hardcoded as ChangeDetectorRef in Autowired).

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
6 changes: 6 additions & 0 deletions packages/core/test/bundling/animations/bundle.golden_symbols.json
View file
Open in desktop
Original file line number Diff line number Diff line change
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
Loading