Overloads with newable signatures not chosen

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.

Daniel Rosenwasser (@DanielRosenwasser)

Daniel Rosenwasser (@DanielRosenwasser) thoughts?

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'.

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?

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.

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.

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.

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:

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.

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.

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?

Wesley Wigham (@weswigham)

Wesley Wigham (@weswigham) is there any action item on our side for this?

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.

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!

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.

Navigation Menu

Repository navigation

Overloads with newable signatures not chosen #24428

4 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Overloads with newable signatures not chosen #24428

Description

Activity

RyanCavanaugh commented on May 29, 2018

rkirov commented on Sep 18, 2018

DanielRosenwasser commented on Sep 24, 2018

rkirov commented on Sep 24, 2018

weswigham commented on Sep 24, 2018

weswigham commented on Sep 24, 2018

rkirov commented on Sep 24, 2018

weswigham commented on Sep 24, 2018

4 remaining items

Goodwine commented on Mar 1, 2019

RyanCavanaugh commented on Mar 13, 2019

weswigham commented on Mar 13, 2019

rkirov commented on Mar 13, 2019

weswigham commented on Mar 13, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions