Skip to content
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.

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

DI - Handle the "InjectFlags" in Injector_.get() #31776

Closed
TomMenga opened this issue Jul 22, 2019 · 13 comments
Closed

DI - Handle the "InjectFlags" in Injector_.get() #31776

TomMenga opened this issue Jul 22, 2019 · 13 comments
Labels
area: core Issues related to the framework runtime core: di feature: insufficient votes Label to add when the not a sufficient number of votes or comments from unique authors feature: votes required Feature request which is currently still in the voting phase feature Issue that requests a new feature state: has PR
Milestone

Comments

@TomMenga
Copy link

🚀 feature request

Relevant Package

This feature request is for @angular/core

Description

At the moment the Injector_.get() implementation doesn't handle the InjectFlags.
I'd like the feature so that I can use the component Injector like this:

@Component(...)
export class SomeComponent {
    // Here I get an instance of Injector_
    constructor(injector: Injector) {
        this.myService = injector.get(SomeService, null, InjectFlags.SkipSelf);
    }
}

Describe the solution you'd like

It looks like you already handle the DeepFlags. refs.ts:342
You probably just need to map and pass the InjectFlags to the resolveDeep() function

class Injector_ implements Injector {
  ...
  get(token: any, notFoundValue: any = Injector.THROW_IF_NOT_FOUND): any {
    ...
    return Services.resolveDep(
        this.view, this.elDef, allowPrivateServices,
        {flags: DepFlags.None, token, tokenKey: tokenKey(token)}, notFoundValue);
  }
}

I don't think I have enough knowledge of the project to submit a PR by myself, so I'm relying on you for this.
Thanks in advance for your work

@AndrewKushnir AndrewKushnir added the area: core Issues related to the framework runtime label Jul 22, 2019
@ngbot ngbot bot added this to the needsTriage milestone Jul 22, 2019
@mhevery mhevery added the feature Issue that requests a new feature label Aug 20, 2019
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Aug 20, 2019
@pmilic021
Copy link

Whats the status on this one? If no one is taking care of it, I could go ahead and try to make a PR for a solution.

@stupidawesome
Copy link

NodeInjector isn't passing the flag here di.ts#L628

export class NodeInjector implements Injector {
  constructor(
      private _tNode: TElementNode|TContainerNode|TElementContainerNode|null,
      private _lView: LView) {}

  get(token: any, notFoundValue?: any): any {
    return getOrCreateInjectable(this._tNode, this._lView, token, undefined, notFoundValue);
  }
}

@kpeterbauer
Copy link

I would call it a bug, not a feature: The provided Injector clearly does not behave as defined by the API documentation:

https://angular.io/api/core/Injector

@dimamarksman
Copy link

Looks like a bug, the type signature says us that there is third option for flag, but under the hood the implementation does not take this option into account.

@johncrim
Copy link

johncrim commented Oct 27, 2020

Definitely a bug IMO - the Injector signature is:

export abstract class Injector {

  /**
   * Retrieves an instance from the injector based on the provided token.
   * @returns The instance from the injector if defined, otherwise the `notFoundValue`.
   * @throws When the `notFoundValue` is `undefined` or `Injector.THROW_IF_NOT_FOUND`.
   */
  abstract get<T>(
      token: Type<T>|InjectionToken<T>|AbstractType<T>, notFoundValue?: T, flags?: InjectFlags): T;
  /**
   * @deprecated from v4.0.0 use Type<T> or InjectionToken<T>
   * @suppress {duplicate}
   */
  abstract get(token: any, notFoundValue?: any): any;

and NodeInjector is:

export class NodeInjector implements Injector {
  constructor(
      private _tNode: TElementNode|TContainerNode|TElementContainerNode|null,
      private _lView: LView) {}

  get(token: any, notFoundValue?: any): any {
    return getOrCreateInjectable(this._tNode, this._lView, token, undefined, notFoundValue);
  }
}

So, NodeInjector implements the deprecated signature, not the current one. I should add that the StaticInjector (which I'm pretty sure is the module injector under Ivy) does correctly implement the Injector API with InjectFlags. ReflectiveInjector and OutletInjector also don't correctly implement the Injector API.

Here's a karma test suite that demonstrates the bug:

import { Component, Injectable, InjectFlags, Injector, NgModule } from '@angular/core';
import { TestBed } from '@angular/core/testing';

@Injectable()
export class FooService {
  constructor(public id: string) {
    console.log('Creating FooService: ', this.id);
  }
}

@Component({
  selector: 'app-c-foo',
  template: '<div>Component that provides a FooService</div>',
  providers: [
    {
      provide: FooService,
      useFactory: () => new FooService('CWithFoo')
    }
  ]
})
export class CWithFooComponent {

  public inheritedFoo: FooService | null;

  constructor(private injector: Injector) {
    // tslint:disable-next-line: no-bitwise
    this.inheritedFoo = this.injector.get(FooService, null, InjectFlags.Optional | InjectFlags.SkipSelf);
  }
}

@Component({
  selector: 'app-parent',
  template: '<app-c-foo></app-c-foo>',
  providers: [
    {
      provide: FooService,
      useFactory: () => new FooService('Parent')
    }
  ]
})
export class ParentComponent { }

@NgModule({
  providers: [
    {
      provide: FooService,
      useFactory: () => new FooService('FooModule')
    }
  ]
})
export class FooModule { }

describe('Element injector', () => {

  it('can SkipSelf to use service from parent Component', () => {

    TestBed.configureTestingModule({
      declarations: [
        CWithFooComponent,
        ParentComponent
      ],
    }).compileComponents();

    const fixture = TestBed.createComponent(ParentComponent);
    const child = fixture.debugElement.children[0].componentInstance as CWithFooComponent;

    expect(child.inheritedFoo.id).toBe('Parent');
  });

  it('can SkipSelf to use service from module', () => {
    TestBed.configureTestingModule({
      declarations: [
        CWithFooComponent
      ],
      imports: [
        FooModule
      ]
    }).compileComponents();

    const fixture = TestBed.createComponent(CWithFooComponent);
    const component = fixture.componentInstance as CWithFooComponent;

    expect(component.inheritedFoo.id).toBe('FooModule');
  });

});

@johncrim
Copy link

johncrim commented Oct 27, 2020

The likely workaround for this is going to be to use the @SkipSelf() decorator and not use the Injector API directly, but that's suboptimal in a lot of cases.

My use case is that I want to provide services at the directive level only if they're not already provided by an ancestor element, using the following:

Usage:

/** Trigger for opening a popup when an element (eg form element) is focused. */
@Directive({
  selector: '[ui-popup-onfocus]',
  providers: [
    provideIfMissing({
      provide: PopupController,
      factory: () => new PopupController()
    })
  ]
})
export class PopupOnFocusDirective {

Implementation, which doesn't work b/c it requires InjectFlags.SkipSelf and Optional:

/**
 * Like a `FactoryProvider`, but the factory function is only called if the
 * dependency is not already provided by a parent injector.
 */
export interface IfMissingFactoryProvider<T> {
  /** The `Type<T>` or `InjectionToken<T>` to inject. */
  provide: Type<T> | InjectionToken<T>;
  /** A factory function to create a T */
  factory: (...args: any[]) => T;
  /** Injectable dependencies that are passed to the `factory` function. */
  deps?: any[];
}

/**
 * Injects an instance of type T if not already provided by a parent.
 *
 * @param provide A `Type<T>` or `InjectionToken<T>`
 * @param factory A factory function to create a T
 * @param deps Injectable dependencies that are passed to the `factory` function.
 */
export function provideIfMissing<T>(
  {
    provide,
    factory,
    deps
  }: IfMissingFactoryProvider<T>): FactoryProvider {

  let factoryDepth = 0;

  function ifMissingFactory(injector: Injector, ...args: any[]): T {
    if (factoryDepth === 0) {
      factoryDepth++; // Protect against recursion
      // tslint:disable-next-line: no-bitwise
      const t: T | null = injector.get<T>(provide, undefined, InjectFlags.Optional | InjectFlags.SkipSelf);
      factoryDepth--;
      if (t !== null) {
        return t;
      }
    }
    return factory(...args);
  }

  return {
    provide,
    useFactory: ifMissingFactory,
    deps: deps ? [ Injector, ...deps ] : [ Injector ]
  };
}

The workaround is for doing the same thing using @SkipSelf is quite a bit less clean - will require constructor modification and injecting custom factories on each class in the chain, instead of just using a shared provider definition.

@matthewjh
Copy link
Contributor

Yes, need this. If the flags are not supported, the implementation needs to throw an error saying so

@johncrim
Copy link

I've found that if I use inject() instead of Injector.get() I'm able to use the InjectFlags arg. At least it seems to work so far.

The limitation of this is that the inject() function only works in factory functions, you can't use it in regular code where Injector can be used. But with some refactoring use of Injector could usually be moved into factory functions.

@johncrim
Copy link

After some more testing, I've found that:

In Ivy w/ Angular 11.0.2:

  • inject(token, InjectFlags) correctly uses the InjectFlags
  • Injector.get(token, defaultValue, InjectFlags) throws away the InjectFlags
    Note that inject() can only be used in factory functions.

In ViewEngine w/ Angular 11.0.2, in Jest tests:

  • Calling inject() from a factory function throws an error: inject() must be called from an injection context
  • Injector.get(token, defaultValue, InjectFlags) still throws away the InjectFlags

This inconsistency is a problem for me b/c we use Jest for testing, and jest-preset-angular is still compiling to ViewEngine (they're working on compiling using Ivy). My (hopefully temporary) workaround is going to be using karma for all the tests that require Ivy injection behavior.

@mrkeathley
Copy link

mrkeathley commented Apr 6, 2021

Any updates on this? I might be underestimating the difficulty but it seems like the NodeInjector could easily implement the correct Injector function. It is currently passing in undefined to getOrCreateInjectable for the InjectorFlags.

export class NodeInjector implements Injector {
  constructor(
      private _tNode: TElementNode|TContainerNode|TElementContainerNode|null,
      private _lView: LView) {}
  get(token: any, notFoundValue?: any): any {
    return getOrCreateInjectable(this._tNode, this._lView, token, undefined, notFoundValue);
  }
}

vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Apr 13, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Apr 13, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Apr 29, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Apr 29, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Apr 29, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Apr 29, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Apr 29, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue May 3, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue May 7, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue May 7, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue May 8, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue May 8, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
@angular-robot angular-robot bot added the feature: votes required Feature request which is currently still in the voting phase label Jun 4, 2021
@angular-robot
Copy link
Contributor

angular-robot bot commented Jun 4, 2021

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

@angular-robot
Copy link
Contributor

angular-robot bot commented Jun 24, 2021

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.

@angular-robot angular-robot bot added the feature: insufficient votes Label to add when the not a sufficient number of votes or comments from unique authors label Jun 24, 2021
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Sep 2, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Oct 21, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Oct 21, 2021
close angular#31776

the InectFlags was defined in both getOrCreateInjectable and Injector, but was ignored in NodeInjector
this PR support getting parent token via injector.get in NodeInjector
vthinkxie pushed a commit to vthinkxie/angular that referenced this issue Oct 22, 2021
The `InjectFlags` argument was defined for both `getOrCreateInjectable()` and `Injector`, but was ignored in `NodeInjector`.
This PR adds support for getting parent token via `NodeInjector.get()`.

close angular#31776
alxhub pushed a commit that referenced this issue Oct 28, 2021
…1592)

The `InjectFlags` argument was defined for both `getOrCreateInjectable()` and `Injector`, but was ignored in `NodeInjector`.
This PR adds support for getting parent token via `NodeInjector.get()`.

close #31776

PR Close #41592
alxhub pushed a commit that referenced this issue Oct 28, 2021
…1592)

The `InjectFlags` argument was defined for both `getOrCreateInjectable()` and `Injector`, but was ignored in `NodeInjector`.
This PR adds support for getting parent token via `NodeInjector.get()`.

close #31776

PR Close #41592
@alxhub alxhub closed this as completed in 65cb2c5 Oct 28, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 28, 2021
Serginho pushed a commit to TuLotero/angular that referenced this issue Jan 20, 2022
…gular#41592)

The `InjectFlags` argument was defined for both `getOrCreateInjectable()` and `Injector`, but was ignored in `NodeInjector`.
This PR adds support for getting parent token via `NodeInjector.get()`.

close angular#31776

PR Close angular#41592
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime core: di feature: insufficient votes Label to add when the not a sufficient number of votes or comments from unique authors feature: votes required Feature request which is currently still in the voting phase feature Issue that requests a new feature state: has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.