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

Type inference for union type passed to overloaded generic function seems broken #20215

Closed
nickjs opened this issue Nov 22, 2017 · 8 comments
Closed
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this
Milestone

Comments

@nickjs
Copy link

nickjs commented Nov 22, 2017

TypeScript Version: 2.7.0-dev.20171121, 2.6.1

Type unions coming specifically from an interface and passed to an overloaded generic constructor (like new Date) seems to break the type inference. Neither of the possible constructor overloads are matched, even though both parts of the type union are supported by the constructor.

Code

interface Foo {
  publishDate: Date | string;
}

const source: Foo = { publishDate: new Date() };
const target = new Date(source.publishDate);

Expected behavior:

target => 2017-11-22T04:08:13.477Z

Actual behavior:

Argument of type 'string | Date' is not assignable to parameter of type 'VarDate'. Type 'Date' is not assignable to type 'VarDate'.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 27, 2017

looking at the spec, seems like Date is a valid argument to Date constructor. (https://www.ecma-international.org/ecma-262/6.0/#sec-date-value).

PRs welcomed. we should also combine the existing overloads to be:

    new(value: number | string | Date): Date;

@mhegazy mhegazy added Help Wanted You can do this Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Bug A bug in TypeScript labels Nov 27, 2017
@mhegazy mhegazy added this to the Community milestone Nov 27, 2017
@nickjs
Copy link
Author

nickjs commented Nov 27, 2017

Hey @mhegazy, thanks for checking into this. You are correct, Date is a valid argument, but that already works in TypeScript, that's not the behavior this issue calls attention to. Simply passing a Date works fine: new Date(new Date()) => Date. What breaks is passing a type union of two compatible types to an overloaded constructor. Using Date here is just an example, as its constructor can accept both string and Date. Passing either of these directly works great. But passing a value of type string | Date where the specific value isn't known at compile time, that's when the compiler breaks on the constructor.

Let me know where I can provide more details! Thanks again.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 27, 2017

Unions are not distributed over calls at the moment. in other words, new Date(x) where x is string | Date, it only works iff we have an overload that accepts string | Date, which does not exist. we do have one for each. so we need to combine them into one overload as noted earlier.

@nickjs
Copy link
Author

nickjs commented Nov 28, 2017

Unions are not distributed over calls at the moment

I think that's the heart of the issue. I'm happy to change the Date constructor to string | number | Date, but that doesn't solve the root cause and wouldn't fix the issue for everything other than Date. Is there already an issue or roadmap item for distributing unions over overloaded calls?

@mhegazy
Copy link
Contributor

mhegazy commented Nov 28, 2017

Please see #14107

@nickjs
Copy link
Author

nickjs commented Nov 28, 2017

Thanks @mhegazy, that context is super helpful. I'm going to close this issue as duplicate of #14107, and open a PR specifically to change the typing on the Date constructor, since that seems to be the best course of action here.

@nickjs nickjs closed this as completed Nov 28, 2017
@mhegazy mhegazy reopened this Nov 28, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 28, 2017

We should combine the two Date signatures nevertheless.

@jakebailey
Copy link
Member

Setting lib to es2015 and above enables this signature:

interface DateConstructor {
    new (value: number | string | Date): Date;
}

Comparing the ES5 and ES6/ES2015 specs, the latter is where Date became valid, so the lib now appears to be accurate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: lib.d.ts The issue relates to the different libraries shipped with TypeScript Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

4 participants