Skip to content

Use typed get method in TestBed class #26491

New issue
New issue
Closed
Closed
Use typed get method in TestBed class#26491
Labels
area: coreIssues related to the framework runtimearea: testingIssues related to Angular testing features, such as TestBedfeatureLabel used to distinguish feature request from other issues
Milestone
Backlog

Description

@michaeljota
michaeljota
opened on Oct 16, 2018

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[x] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question
[ ] Other... Please describe:

Current behavior

The get static method of TestBed class does not return a typed object.

const service = TestBed.get(MyService); // service is typed as any

Expected behavior

The get static method should return a typed object.

const service = TestBed.get(MyService); // service should be typed as MyService

Minimal reproduction of the problem with instructions

Look examples.

What is the motivation / use case for changing the behavior?

This would improve type safety.

Environment


Angular version: 6.1.9

Others:

A possible implementation using new helper types from TypeScript would be:

    static get<T extends new (...args: any[]) => any>(token: T, notFoundValue?: T | InstanceType<T>): InstanceType<T>;

I'm not sure if the notFoundValue should be a constructor or an instance, so I typed it as both, but fell free to update it as is actually needed. I tested that signature and it actually returns a typed object as wanted.

Activity

  1. IgorMinar
    added
    area: testingIssues related to Angular testing features, such as TestBed
    area: coreIssues related to the framework runtime
    on Oct 16, 2018
  2. ngbot
    added this to the needsTriage milestone on Oct 16, 2018

  3. trotyl commented on Oct 17, 2018

    @trotyl
    trotyl
    on Oct 17, 2018
    Contributor

    Relates to #18695, #23611, #24151

  4. harshabada commented on Oct 17, 2018

    @harshabada
    harshabada
    on Oct 17, 2018

    did u added the above service in providers

  5. mhevery commented on Oct 25, 2018

    @mhevery
    mhevery
    on Oct 25, 2018
    Contributor

    A PR would be great!

  6. michaeljota commented on Nov 13, 2018

    @michaeljota
    michaeljota
    on Nov 13, 2018
    Author

    @mhegazy I'll try to get into it. No promises.

  7. alxhub
    added
    featureLabel used to distinguish feature request from other issues
    on Nov 27, 2018
  8. ngbot
    modified the milestones: needsTriage, Backlog on Nov 27, 2018

  9. Goodwine commented on Jan 22, 2019

    @Goodwine
    Goodwine
    on Jan 22, 2019

    @michaeljota hiya, I was facing this same issue today and found your report, are you working on a PR? otherwise I'm thinking I can try to send one, just didn't want to step over

  10. michaeljota commented on Jan 22, 2019

    @michaeljota
    michaeljota
    on Jan 22, 2019
    Author

    Hi @Goodwine. I haven't get a chance to look at this. You can give it a look if you want. Thanks!

  11. Goodwine commented on Jan 23, 2019

    @Goodwine
    Goodwine
    on Jan 23, 2019

    My proposal is to use these types -

    get<T>(token: InjectionToken<T>): T;
get<T>(token: Type<T>): T;
get(token: T): unknown;
get<T1, T2>(token: InjectionToken<T1>, notFoundValue: T2): T1|T2;
get<T1, T2>(token: Type<T1>, notFoundValue: T2): T1|T2;
get<T2>(token: T, notFoundValue: T2): unknown;

    The reason for using unknown when the value isn't either an InjectionToken or a Type is because the desired type is actually an abstract class, and this will force the user to cast to the correct value.

    There is one problem with this approach though. What if the user wants to use the abstract class interface? Or what if the user knows the real value? Unfortunately this is kinda annoying to do in TS because if we have an abstract class Foo, the return value would be typeof Foo. We're only able to get away with it with concrete classes because angular has this fancy type Type which extends anything with a constructor so for a concrete class Bar, the inferred value is Type<Bar> which allows for easily returning of type Bar rather than typeof Bar

    Perhaps the return value should be never so that the user isn't even allowed to cast the return value and instead they would have to cast the argument. For example:

    // bad?
  const foo = TestBed.get(Foo); // returns "never"
// good?
  const foo = TestBed.get(Foo as Type<Foo>) // returns type Foo

    Problem is that this could be anything and the user could cast to whatever they want, so maybe returning unknown is best?

  12. michaeljota commented on Jan 23, 2019

    @michaeljota
    michaeljota
    on Jan 23, 2019
    Author

    I would consider an approach that reduce type casting, as I think is just about that.

  13. michaeljota commented on Jan 23, 2019

    @michaeljota
    michaeljota
    on Jan 23, 2019
    Author

    @Goodwine What do you think about something like this:

        get<T>(token: T | InjectionToken<T>, notFoundValue?: T): T  extends new (...args: any[]) => any ? InstanceType<T> : T;

    I think this would reduce type casting. I'm want to be sure that I understand the usage of Type<T> in your examples, but I think this can be done with conditional typing.

  14. 10 remaining items

  15. michaeljota commented on Jan 24, 2019

    @michaeljota
    michaeljota
    on Jan 24, 2019
    Author

    Why would you want to get null if not found? I'm trying to understand that scenario.

  16. Goodwine commented on Jan 24, 2019

    @Goodwine
    Goodwine
    on Jan 24, 2019

    That's just to support the existing use of get(NgZone, null), I'd rather have one type as mentioned above.

    To be honest I'd like to get rid of the second param for return this when the other was not found, a variadic parameter for option (the way it's currently used) would be OK with me tho.

  17. Goodwine commented on Mar 13, 2019

    @Goodwine
    Goodwine
    on Mar 13, 2019

    Talking with the angular team (I'm not part of the angular team), I'll actually send PRs in the following order:

    1. Add @deprecate to current function signature and add same signature as Injector.get, however it would still return any.
    2. Add set of types that will eventually be used, even if they aren't used yet.
    3. Insert breaking changes that could land in a future major version, but definitely not Angular 8
  18. jasonaden
    added a commit that references this issue on Apr 2, 2019

    feat(core): Add "AbstractType<T>" interface (#29295)

    afd4a4e
  19. IgorMinar
    added a commit that references this issue on Apr 4, 2019

    fix(core): Deprecate TestBed.get(...):any (#29290)

    609024f
  20. wKoza
    added 2 commits that reference this issue on Apr 17, 2019

    feat(core): Add "AbstractType<T>" interface (angular#29295)

    fa7a19b

    fix(core): Deprecate TestBed.get(...):any (angular#29290)

    36551f8

  21. Goodwine commented on Aug 19, 2019

    @Goodwine
    Goodwine
    on Aug 19, 2019

    Per discussion with @vikerman, the Angular team can't accept such a breaking change at this point, so the approach we'll now take is to create a new method TestBed.inject which will be type-safe. And deprecate TestBed.get.

  22. mhevery
    added a commit that references this issue on Aug 29, 2019

    feat(core): Introduce TestBed.inject to replace TestBed.get (#32200)

    3aba7eb
  23. sabeersulaiman
    added a commit that references this issue on Sep 6, 2019

    feat(core): Introduce TestBed.inject to replace TestBed.get (angular#…

    7514f3c
  24. matsko
    closed this as completedin a85eccdon Sep 12, 2019

  25. angular-automatic-lock-bot commented on Oct 13, 2019

    @angular-automatic-lock-bot
    angular-automatic-lock-botBot
    on Oct 13, 2019

    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.

  26. angular-automatic-lock-bot
    locked and limited conversation to collaborators on Oct 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: coreIssues related to the framework runtimearea: testingIssues related to Angular testing features, such as TestBedfeatureLabel used to distinguish feature request from other issues

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions