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

Return type inference broken for types with static members in ts 2.5; later fixed #20102

Open
alexeagle opened this issue Nov 17, 2017 · 5 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@alexeagle
Copy link
Contributor

Angular's dependency injection looks something like this:

interface Type<T> extends Function {
  new (...args: any[]): T;
}

function get<T>(token: Type<T>, notFoundValue?: T): T {
  return null as any as T;
}

class Token0 {
  a = 1;
}
class Token1 {
  static a = 1;
}

let t0 = get(Token0);  // t0 always inferred as Token0
let t1 = get(Token1);  // TS 2.5.3: t1 inferred as `{}` 
                       // TS 2.6.1 t1 inferred as `Token1`

As stated in those comments, starting in TS 2.5 we get the wrong type when a user asks the Injector for a Token with a static member. It's later fixed in TS 2.6.

Workaround is to give an explicit type argument to get, eg Angular users should repeat the type like Injector.get<Token1>(Token1)

There may be no action required here other than asking users to upgrade to TS 2.6 - but sadly Angular doesn't support it yet because we need to upgrade all the places we depend on first. Filing this issue mostly so we have a place to point to in the Angular changelog.

@DanielRosenwasser DanielRosenwasser added the In Discussion Not yet reached consensus label Nov 17, 2017
@aluanhaddad
Copy link
Contributor

@alexeagle you could work around this temporarily by using the following signature for get.

declare function get<T>(token: new (...args: any[]) => T, notFoundValue?: T): T;

where in 2.5.3

// tslint:disable
interface Type<T> extends Function {
  new(...args: any[]): T;
}

declare function get<T>(token: new (...args: any[]) => T, notFoundValue?: T): T;

class Token0 {
  a = 1;
}
class Token1 {
  static a = 1;
}

let t0 = get(Token0); // Token0
let t1 = get(Token1); // Token1

@alexeagle
Copy link
Contributor Author

Thanks for the suggestion @aluanhaddad !
It turns out it's not as simple as my repro: https://github.com/angular/angular/blob/69c53c3e03c7a8bb2260496c9aa32925cd2d59b4/packages/core/src/di/injector.ts#L60
changing the declaration of get causes some callsites to fall through to the second, any-typed overload. So it has some ripple effects in Angular's code. Maybe this could still be patched up to work.
/cc @IgorMinar

@RyanCavanaugh
Copy link
Member

@DanielRosenwasser what does "In Discussion" mean here? Is there a concrete suggestion to be considered?

@mhegazy mhegazy added Bug A bug in TypeScript Needs Investigation This issue needs a team member to investigate its status. and removed In Discussion Not yet reached consensus labels Nov 20, 2017
@mhegazy mhegazy self-assigned this Nov 20, 2017
@DanielRosenwasser
Copy link
Member

@RyanCavanaugh the suggestion is backporting whatever changes were applied to 2.5.x, which we don't typically do. The bug is already fixed, which is why I didn't mark it as such.

@aluanhaddad
Copy link
Contributor

@alexeagle you should be able to add an overload that takes the existing types to preserve the current behavior. The new signature will need to be declared first since it is more specific.

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

@mhegazy mhegazy added this to the TypeScript 2.9 milestone Apr 12, 2018
@mhegazy mhegazy modified the milestones: TypeScript 3.0, Future Jul 2, 2018
@RyanCavanaugh RyanCavanaugh removed the Needs Investigation This issue needs a team member to investigate its status. label Mar 7, 2019
@RyanCavanaugh RyanCavanaugh removed the Needs Investigation This issue needs a team member to investigate its status. label Mar 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants