Overloads with newable signatures not chosen

[rkirov]

TypeScript Version: 3.0.0-dev.201xxxxx

Search Terms: newable signature

Code

declare function construct<T>(ctor: new(...args: any[]) => T): T;
declare function construct<T>(ctor: Function): T;

class C {
  constructor(public x: string) {};
}

const instance = construct(C);
instance.x;

Expected behavior:
The first overload is chosen, instance is of type C, thus no error at instance.x.

Actual behavior:
Second overload is chosen, instance is of type {} (as per underspecified generics rules), error at instance.x because {} has no x.

Playground Link: http://www.typescriptlang.org/play/#src=declare%20function%20construct%3CT%3E(ctor%3A%20new(...args%3A%20any%5B%5D)%20%3D%3E%20T)%3A%20T%3B%0D%0Adeclare%20function%20construct%3CT%3E(ctor%3A%20Function)%3A%20T%3B%0D%0A%0D%0Aclass%20C%20%7B%0D%0A%20%20constructor(public%20x%3A%20string)%20%7B%7D%3B%0D%0A%7D%0D%0A%0D%0Aconst%20instance%20%3D%20construct(C)%3B%0D%0Ainstance.x%3B%0D%0A

Some odd things that I don't fully understand:

• only happens when we turn on strictFunctionTypes
• doesn't happen if C has no ctor arguments.
• if there is a single overload for the newable signature only, it is chosen fine.

Note this is based on real world code in angular typings.
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/8091b4653666317dc94a3ede9f84d3114c67db81/types/angular/index.d.ts#L1394

One workaround is to change typings not to use underspecified Function type.

[RyanCavanaugh]

@DanielRosenwasser thoughts?

[rkirov]

We keep seeing this bug as we are rolling out --strictFunctionTypes to Angular users. Other places where it occurs is:

// https://github.com/angular/angular/blob/master/packages/core/src/type.ts#L25
export interface Type<T> { new (...args: any[]): T; }

// https://github.com/angular/angular/blob/master/packages/core/src/di/injector.ts#L71
declare function get<T>(token: Type<T>): T;
// https://github.com/angular/angular/blob/master/packages/core/src/di/injector.ts#L76
declare function get<T>(token: any): any;

class C {
  constructor(a: string) {}
}

let f = get(C);
f.boom; // should fail if f is of type C, not any

With --strictFnTypes f is of type 'any', while without the flag it is of type 'C'.

[DanielRosenwasser]

I don't 100% know why, but I think this is because of that crappy preliminary subtype pass that we do on overload resolution. It's sometimes helpful though.

If you switch to

export interface Type<T> { new (...args: never[]): T; }

that should work as you expect though. Is that a reasonable workaround?

[rkirov]

In the realm of workarounds, leave as it is, is an ok workaround, because you just get some more 'any's. Changing the angular core types would be harder, probably something else will error and 'never[]' is quite unorthodox, though I see the reasoning.

I am more interested in getting a proper fix on the roadmap in future releases.

Also the 'constructor' pattern is used quite often for frameworks that want to accept any consturctor. Here it is in angular material - https://github.com/angular/material2/blob/master/src/lib/core/common-behaviors/constructor.ts.

[weswigham]

I think this is because of that crappy preliminary subtype pass that we do on overload resolution. It's sometimes helpful though.

This is precisely the cause. new(x: string) => C isn't a subtype of new(...args: any[]) => T because any isn't a subtype of string (strict argument variance inverts the relationship for the parameter, and while any is assignable to string, it is not a subtype), while new(x: string) => C is easily a subtype of Function, structurally.

[weswigham]

I think Daniel's suggestion is actually the appropriate fix to make your code strictFunctionTypes safe. And it emphasizes that while you are taking a thing with any number of constructor arguments, you can never safely actually provide any arguments for that constructor in the function's implementation (since nothing is assignable to never sans never itself) since you're not validating what those types are.

[rkirov]

I see, so using never[] is the principled answer for --strict users and not just a workaround. In that case, I will file bugs with angular and material to fix this.

Would you be willing to add something like Constructable to the core libs? I am seeing this all over the place, because it is so core to mixins and DI, and basically every one has used 'any' instead of 'never'.

export type Constructable<T> = new (...args: never[]) => T;

References for mixins:

• https://blogs.msdn.microsoft.com/typescript/2017/02/02/announcing-typescript-2-2-rc/
• https://blog.mariusschulz.com/2017/05/26/typescript-2-2-mixin-classes

Finally, I still think there is a bug (maybe more minor that originally thought), because why removing the 'any' overload succeeds even with --strict on.

// https://github.com/angular/angular/blob/master/packages/core/src/type.ts#L25
export interface Type<T> { new (...args: any[]): T; }

// https://github.com/angular/angular/blob/master/packages/core/src/di/injector.ts#L71
declare function get<T>(token: Type<T>): T;

class C {
  constructor(a: string) {}
}

let f = get(C);
f.boom; // f is inferred as C and there is type error here.

[weswigham]

Finally, I still think there is a bug (maybe more minor that originally thought), because why removing the 'any' overload succeeds even with --strict on.

Yeah, the call is still valid with any, we just prioritize overloads whose arguments are proper subtypes to arguments which are merely assignable during call resolution. any is flexible for assignability (where it's both a top type and a bottom type), but as far as subtyping goes (which again, matters in this case since calls which resolve using subtypes are preferred), only functions as an unknown-like top. In strictFunctionTypes we become very pedantic about variance when comparing signatures (without it we check either direction and if either direction is fine we say it's OK), and so using the correct bounds type (never or unknown/any) for a given position becomes very important for proper subtype relationships.

[rkirov]

Got it, thanks for the explanation. To summarize:

• deciding which overload is used is based on 'subtyping'
• once chosen it is checked by 'assignability'
• 'any' behaves like 'unknown' for 'subtyping' purposes, but has different 'assignability' rules.

My surprise comes from expecting the following property "If there are two fn overloads and I observe the second one is chosen, then by removing the second one, I should see a type error with the first one". Now I see why that is not a property that holds.

When I want to experimentally test an 'assignability' relation between S and T, I do the following:

function f(s: S, t: T) {
  s = t;
  t = s;
}

Is there any similar black-box testing I can do to observe the 'subtyping' relation between TS types?

[weswigham]

Is there any similar black-box testing I can do to observe the 'subtyping' relation between TS types?

Y'know, I don't think there is, actually. Narrowing and overload resolution are the only places we use the subtype relationship, which mostly only differs from assignability in that any is treated much the same as unknown (and some things with numbers not being subtypes of numeric enums, despite them being assignable).

It won't cause an error, but instantiations of unions can cause subtype reduction to occur (ie, where the union simplifies and removes superflous subtypes). Eg, if you write ("a" | string)[] you just get string[] because "a" is a subtype of string. This is likely the most visible use of the subtype relationship.

[Goodwine]

Conditional types instead of overloads as a workaround for now? It seems to me that the overloads buggy on some cases like the one described here

declare function construct<
  T extends Function | (new (...args: any[]) => any)
>(
    ctor: T,
    ): T extends new (...args: any[]) => infer U ? U : unknown;

[example]

I'm on the opinion that Function should return unknown instead of T :)

but we could do something like... T extends Newable<infer U> ? U : T extends () => infer U ? U : unknown or something like that.. maybe?

[RyanCavanaugh]

@weswigham is there any action item on our side for this?

[weswigham]

I don't think so? The original issue was phrased as a kind of question - a change was proposed to make the OP's code typecheck, and the reason why it is like it is was given.

[rkirov]

Yes, I had a poor understanding of how overloads were chosen. The existence of 'subtyping' vs 'assignability' relation checks was news to me. I find it surprising, but not buggy. Thanks for explaining Wes!

[weswigham]

Yeah, we know overload resolution is really complex with a lot of nuance that you don't usually need to know about. Some patterns are hard to type without them, but we recommend using simple union types (and maybe conditionals) where possible instead.