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

Inferred type incomplete, when inferring parameters type on surcharged constructors. #59812

Closed
denis-migdal opened this issue Aug 31, 2024 · 13 comments
Labels
Unactionable There isn't something we can do with this issue

Comments

@denis-migdal
Copy link

denis-migdal commented Aug 31, 2024

πŸ”Ž Search Terms

None

πŸ•— Version & Regression Information

Tested on the nightly build.

⏯ Playground Link

tsx function foo(a: boolean): void function foo(a: number): void function foo(a: boolean|number) {} type InferArgs<T> = T extends (...args: infer U) => void ? U : never; type Args = InferArgs<typeof foo... Playground Link

πŸ’» Code

function foo(a: boolean): void
function foo(a: number): void
function foo(a: boolean|number) {}

type InferArgs<T> = T extends (...args: infer U) => void ? U : never;

type Args = InferArgs<typeof foo>;

let a: Args = [true];

πŸ™ Actual behavior

When inferring the function parameters, only one of the function signature is used.
Here, Args is [number], which raises an error in the last line.

πŸ™‚ Expected behavior

When inferring the function parameters, the union of the different signatures should be returned.
Here, Args should be [number]|[boolean].

Additional information about the issue

No response

@jcalz
Copy link
Contributor

jcalz commented Aug 31, 2024

You didn’t provide any search terms. Did you search for existing issues before filing this? Also, β€œsurcharged” isn’t a term that makes much sense here but from context I gather you mean β€œoverloaded”.

Duplicate #43731

@MartinJohns
Copy link
Contributor

This is also in the documentation: Inferring Within Conditional Types

When inferring from a type with multiple call signatures (such as the type of an overloaded function), inferences are made from the last signature (which, presumably, is the most permissive catch-all case). It is not possible to perform overload resolution based on a list of argument types.

@denis-migdal
Copy link
Author

denis-migdal commented Aug 31, 2024

You didn’t provide any search terms.

Sorry for that, I was in a train with very slow/limited Internet access.

Also, β€œsurcharged” isn’t a term that makes much sense here but from context I gather you mean β€œoverloaded”.

Indeed, bad translation (in French we say "surcharge").

This is also in the documentation: Inferring Within Conditional Types

So according to the documentation :

When inferring from a type with multiple call signatures (such as the type of an overloaded function), inferences are made from the last signature (which, presumably, is the most permissive catch-all case).

The last signature being :

function foo(a: boolean|number)

Shouldn't Args be at least [boolean|number] ?
It makes no sense for it to be sometimes the first signature, and sometimes the second signature.

@jcalz
Copy link
Contributor

jcalz commented Aug 31, 2024

"I was in a train" So, um, wait until you're off the train to file issues? Maybe? I don't know how to process this response.

"The last signature being" no, that's the implementation. It's not a call signature (try calling the function with Math.random()<0.5 ? true: 123 as an argument) If you want that call signature you need to add it explicitly.

@denis-migdal
Copy link
Author

If you want that call signature you need to add it explicitly.

Well, I don't want this to be a call signature, but the implementation signature to be used during inferences as it makes the most sense.

The documentation is also a little strange :

the last signature (which, presumably, is the most permissive catch-all case).

It is the implementation signature that is the catch-all case, not the last call signature.

@MartinJohns
Copy link
Contributor

The documentation refers to call signatures. You can't call the implementation signature. But I guess the wording could be clearer.

It is the implementation signature that is the catch-all case, not the last call signature.

That depends on your code. In your example there is no "catch-all" call signature. But you could add one:

function foo(a: boolean): void
function foo(a: number): void
function foo(a: boolean | number): void
function foo(a: boolean|number) {}

Now you have a catch-all call signature, and your function could be called with a value typed boolean | number.

@denis-migdal
Copy link
Author

denis-migdal commented Aug 31, 2024

That depends on your code. In your example there is no "catch-all" call signature. But you could add one:
[...]
Now you have a catch-all call signature, and your function could be called with a value typed boolean | number.

I think I see 3 issues for this.

The first one is that the third signature would be callable, so would enable to call e.g. foo(true, 3). When we only want this signature for inferences.

The second one is the code duplication between the last call signature and the implementation signature (code duplication is never a great thing).

The third one is that if we wish to use it on existing functions, we'd have to declare it for every functions which can be really time-consuming, with the risk of forgetting one, or if one function is modified, to forget to update the declaration.

Knowing that, without overload, TS already generates a signature from the implementation signature.
Could it be possible, with overload, to also generate one from the implementation (but non-callable) ?

For example, if we assume that TS generates a list of signatures, could we assume that e.g. the first one is non-callable (if other signatures are available), and reserve it for the implementation signature and inferences ? If empty (e.g. no implementation signature known), when doing inferences, could either use the last signature from the list (current behavior), or could even be computed as the union of the other signatures ?

Technically I'd assume this would be possible ?

@jcalz
Copy link
Contributor

jcalz commented Aug 31, 2024

So is this now some kind of feature request? TS is behaving as designed and we’re talking about… implementation signature changes? What’s the status of this as a bug report?

@denis-migdal
Copy link
Author

So is this now some kind of feature request? TS is behaving as designed and we’re talking about… implementation signature changes? What’s the status of this as a bug report?

Currently it seems the issue is a design issue. I think you consider them to be feature requests ?

@jcalz
Copy link
Contributor

jcalz commented Aug 31, 2024

I really don't know, and I'm not a member of the TS team, just a nosy annoying interested bystander, so nothing I say is authoritative. You can feel free to ignore me and just wait for the TS team to get involved. But, I'd say if you don't want this to be closed as just a duplicate of #43731 or others, you should edit it so it uses the feature request template and be specific and clear about what the feature request is, searching first for existing issues with similar requests, to help the TS team triage.

My guess is that they would just decline a suggestion to expose the implementation signature the way you're asking for. Function types don't encode implementation information, they encode call signature information. Declarations for overloads don't mention the implementation, and they shouldn't have to know anything about the implementation (or else TS's library files for JS would need to have all kinds of added information in them). And what good would it do to consult the implementation signature when the function is not callable that way? If you tell me Parameters<typeof foo> is [boolean | number] and I can't call foo(Math.random()<0.5?true:123) then I'm not going to be happy. Personally I'd expect someone who runs into this discrepancy with overloads to ask for #14107 with the hope that you'd get [boolean] | [number] or something like it.

Anyway I think I've said all I can say here so I will bow out and wait to hear what the TS team says. Good luck!

@denis-migdal
Copy link
Author

Thanks for your answer.

Personally I'd expect someone who runs into this discrepancy with overloads to ask for #14107 with the hope that you'd get [boolean] | [number] or something like it.

I agree. I assumed it was not possible as the doc stated:

It is not possible to perform overload resolution based on a list of argument types.

And hard to fix, as the mentioned issues are 5 to 7 years old, without being addressed, while it the issue seems quite real (TS seems to have a lot of such issues, it is kind of frustrating).

Indeed, as you stated :

If you tell me Parameters<typeof foo> is [boolean | number] and I can't call foo(Math.random()<0.5?true:123) then I'm not going to be happy.

You are quite right. It is even more true if we have 2 arguments : [boolean | number, boolean | number], while you can't call foo(true, 0) would indeed be incoherent.

However, I saw that the type has the different callable signatures :

type c = {
    (a: boolean): void;
    (a: number): void;
}

So TS seems to have all required information to perform a correct inference.

But I think I got the issue.

Possible solution 1:

For a simple inference, we could simply try it for each callable signature, and return an union of the inferred types.
But with more complex inference, we could have the same issue as you presented :

type Args<T> = T extends (a: infer A, b: infer B) => infer C ? [A,B, C] : never;

Could give [number|boolean, number|boolean, void]. But it'd be technically correct as we didn't returned the function signature, but a tuple of the possible types for its parameters and return value.

Possible solution 2:

Else, the union could be made latter, by making an union of the results, e.g. returning : [number, number, void]|never|[boolean, boolean, void]. But I assume it might pose some performances issues if there are lot of nested inference ?

Possible solution 3:

Adding to TS the types from the solution given inside the issue you just posted:

Playground Link

@RyanCavanaugh RyanCavanaugh added the Unactionable There isn't something we can do with this issue label Sep 5, 2024
@RyanCavanaugh
Copy link
Member

I don't see anything here that isn't addressed in other issues.

And hard to fix, as the mentioned issues are 5 to 7 years old, without being addressed, while it the issue seems quite real (TS seems to have a lot of such issues, it is kind of frustrating).

Every issue you encounter leads to an open issue rather than a closed one, interesting observation πŸ™‚

Image

@denis-migdal
Copy link
Author

Every issue you encounter leads to an open issue rather than a closed one, interesting observation πŸ™‚

The interesting observation is that lot of theses issues are rather old than new, with multiples instances, and often due to design choices... some issues never got any clear answer whether they'll be fixed one day, its priority level, what prevent/delay a fix, etc.

I have so much fun wasting time on "unsound" stuff, and trying to find dirty workarounds if any.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Unactionable There isn't something we can do with this issue
Projects
None yet
Development

No branches or pull requests

4 participants