Skip to content

[Bug] Parameters of callback not inferred when passed to overloaded function expecting different number of params #31867

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

Closed
AnyhowStep opened this issue Jun 12, 2019 · 6 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Jun 12, 2019

TypeScript Version: 3.4.1, 3.5.1

Search Terms:

parameter, infer, generic, callback

Code

//--noImplicitAny
class Clazz<T> {
    /**
        MUST NOT switch the declarations of `foo()`.
        Run-time logic uses first `foo()` when
        `callback.length <= 3`.
        Run-time logic uses second `foo()` when
        `callback.length == 4`.
    */
    foo(callback: (a: "A", b: "B", t: T) => void): void;
    foo(callback: (a: "A", b: "B", c: "C", t: T) => void): void;
    foo() {
        console.log("blah");
    }
}
const clazz = new Clazz<"qwerty">();
clazz.foo((a, b, t) => {
    //OK!
    //a : "A"
    //b : "B"
    //t : "qwerty"
});
clazz.foo((a, b, c, t) => {
    //Expected: a : "A", b : "B", c : "C", t : "qwerty"
    //Actual  :
    //+ a implicitly has any type
    //+ b implicitly has any type
    //+ c implicitly has any type
    //+ t implicitly has any type
});

//If you switch the declarations of `foo()`
class Clazz2<T> {
    /**
        If you switch the declarations of `foo()`...
    */
    foo(callback: (a: "A", b: "B", c: "C", t: T) => void): void;
    foo(callback: (a: "A", b: "B", t: T) => void): void;
    foo() {
        console.log("blah");
    }
}
const clazz2 = new Clazz2<"qwerty">();
clazz2.foo((a, b, t) => {
    //Expected:
    //a : "A"
    //b : "B"
    //t : "qwerty"
    
    //Actual:
    //a : "A"
    //b : "B"
    //t : "C" <-- Undesirable
});
clazz2.foo((a, b, c, t) => {
    //OK!
    //a : "A"
    //b : "B"
    //c : "C"
    //t : "qwerty"
});

Expected behavior:

  • Callback of clazz.foo((a, b, c, t) => {}) should be inferred correctly
  • TS should select second overload of foo()

Actual behavior:

  • Callback of clazz.foo((a, b, c, t) => {}) not inferred correctly (not expected)
  • TS selects second overload of foo() (as expected)

Playground Link: Playground

Related Issues:

#31146 somewhat related but they don't have overloads


#21525 has overloads where both overloads have the same number of parameters. However, I'm pretty sure my case is different. In that issue, TS selected the first overload and not the second one.

In my case, TS selected the second overload but doesn't realize the parameters of the callback are "A", "B", "C", "qwerty".

From this comment #21525 (comment) , I've tried variations on conditional typing but nothing seems to work.


My current use-case is wrapping express and its middleware feature. Each middleware may have arguments of length 0,1,2,3,4.

If the length is 0-3, it is a request handler.
If the length is 4, it is an error handler.
Otherwise, it is ignored.

So, I need the overload with 3 params to be the first overload.
And the overload with 4 params to be the second overload.


It's weird that it knows what overload is correct, but can't infer the parameters. It's not like my parameters have conditional types or anything complex.

@AnyhowStep
Copy link
Contributor Author

Shorter repro,

//--noImplicitAny
function foo<T>(callback: (a: "A", b: "B", t: T) => void): void;
function foo<T>(callback: (a: "A", b: "B", c: "C", t: T) => void): void;
function foo() {
    console.log("blah");
}
foo<"qwerty">((a, b, t) => {
    //OK!
    //a : "A"
    //b : "B"
    //t : "qwerty"
});
foo<"qwerty">((a, b, c, t) => {
    //Expected: a : "A", b : "B", c : "C", t : "qwerty"
    //Actual  :
    //+ a implicitly has any type
    //+ b implicitly has any type
    //+ c implicitly has any type
    //+ t implicitly has any type
});

Again, it knows the second foo<"qwerty">() is referring to the second overload,
it just can't infer the parameter types.

Playground

@AnyhowStep
Copy link
Contributor Author

Even shorter repro. Seems like generics aren't causing the issue,

//--noImplicitAny
function foo(callback: (a: "A") => void): void;
function foo(callback: (a: "A", b: "B") => void): void;
function foo() {
    console.log("blah");
}
foo((a) => {
    //OK!
    //a : "A"
});
foo((a, b) => {
    //Expected: a : "A", b : "B"
    //Actual  :
    //+ a implicitly has any type
    //+ b implicitly has any type
});

Playground

@AnyhowStep AnyhowStep changed the title [Bug] Parameters of callback not inferred when passed to overloaded generic method [Bug] Parameters of callback not inferred when passed to overloaded function expecting different number of params Jun 12, 2019
@AnyhowStep
Copy link
Contributor Author

Seems like the same thing is happening with @types/express,

image

@ahejlsberg
Copy link
Member

ahejlsberg commented Jun 12, 2019

Let me first say that's a pretty terrible API. Having different types in the same positions of a callback based on measuring the number of parameters in the callback just sounds like a recipe for confusion.

It wouldn't be easy to fix this. Generally overload resolution works by checking whether the types of the arguments are compatible with the signature. But to find the type of the lambda argument we need to contextually type the parameters, and once we do that we can't undo it. We'd need to somehow disqualify the signature before we contextually type the lambda parameters. That would pretty much require a separate additional phase in overload resolution.

@AnyhowStep
Copy link
Contributor Author

I agree that it's terrible API design. I had to dig into the source code to see what it's actually doing =/

For now, I've added an extra method that uses a different name, which only takes the error handler.
Side-stepping the overload problem, basically =x

@ahejlsberg
Copy link
Member

I'm going to close this as a design limitation.

@ahejlsberg ahejlsberg added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

2 participants