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

callbacks should select the right overload #35501

Closed
millsp opened this issue Dec 4, 2019 · 6 comments · Fixed by #42620 · May be fixed by #52944
Closed

callbacks should select the right overload #35501

millsp opened this issue Dec 4, 2019 · 6 comments · Fixed by #42620 · May be fixed by #52944
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@millsp
Copy link
Contributor

millsp commented Dec 4, 2019

TypeScript Version: 3.8.0-dev.20191121

Search Terms:

callback overload mismatch

Code

declare function fn(x: string): string;
declare function fn(x: string[]): string;

declare function map<A, R>(fn: (item: A) => R, list: A[]): R[];

const mapped = map(fn, ['1']) // error

Expected behavior:

TypeScript does infer A as string and could choose the right overload fn(x: string), decline the ones that don't fit.

Actual behavior:

Checks the last overload only, so it shows an error.

Playground Link: http://www.typescriptlang.org/play/?ssl=7&ssc=1&pln=1&pc=1#code/CYUwxgNghgTiAEAzArgOzAFwJYHtVNQAoAPALngGcMYtUBzASnKpvoG4AoUSWBFdbHgIlm1WnQDaAXSaUx7Dl3DQ4SNJlz4AtlAAOAHgCCAGngAlAHyFEqcoSwYQW8oYbwAvBfOmIWKi+lZM2lODjA8KngdXV0QYA8ovWtUUwkAcgBGNJkOIA

Related Issues:

@millsp
Copy link
Contributor Author

millsp commented Dec 4, 2019

Another example, where both overloads would match

declare function fn(x: string): string;
declare function fn(x: string): number;

declare function map<A, R>(fn: (item: A) => R, list: A[]): R[];

const mapped = map(fn, ['1'])

mapped should be of type number[] | string[]

@millsp
Copy link
Contributor Author

millsp commented Dec 4, 2019

Here's my current workaround

declare function fn(x: string): string;
declare function fn(x: string[]): string;

declare function map<A, R>(fn: (item: A) => R, list: A[]): R[];

const mapped = map((x) => fn(x), ['1'])

Wrapping fn does get TS to choose the right overload

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Dec 5, 2019
@RyanCavanaugh
Copy link
Member

Generic inference is not able to do overload resolution, though your examples all don't need overloads in the first place.

@millsp
Copy link
Contributor Author

millsp commented Dec 5, 2019

The examples are silly yes, but they do show how overloads could be improved. I get these scenarios quite often (on ramda especially). So for now, I wrap with () => which does the trick, so I thought this could have been solved easily.

@jamiebuilds
Copy link

Here is the simplest possible replication of this I could come up with: [Playground]

interface Callback {
  (value: number): any
  (value: string): any
}

function fn(callback: Callback) {
  // ...
}

fn(value => {
  // ^ Parameter 'value' implicitly has an 'any' type.
})
Here is a real world example: [Playground]
interface Callback<T> {
  (error: null, result: T): unknown
  (error: Error, result: null): unknown
}

interface Task<T> {
  (callback: Callback<T>): unknown
}

export function series<T>(tasks: Task<T>[], callback: Callback<T[]>): void {
  let index = 0
  let results: T[] = []

  function next() {
    let task = tasks[index]
    if (!task) {
      callback(null, results)
    } else {
      task((error, result) => {
        //  ^ Parameter 'error' implicitly has an 'any' type.
        //         ^ Parameter 'results' implicitly has an 'any' type.
        if (error) {
          callback(error, null)
        } else {
          results.push(result)
          next()
        }
      })
    }
  }
  next()
}

series([
  cb => setTimeout(() => cb(null, 1), 300),
  cb => setTimeout(() => cb(null, 2), 200),
  cb => setTimeout(() => cb(null, 3), 100),
], (error, results) => {
  // ^ Parameter 'error' implicitly has an 'any' type.
  //       ^ Parameter 'results' implicitly has an 'any' type.
  if (error) {
    console.error(error)
  } else {
    console.log(results)
  }
})

@jcalz
Copy link
Contributor

jcalz commented Nov 17, 2021

Just to be clear: Note that even though #42620 is listed above as closing this issue, @weswigham says it only fixed the comment before this one, not the whole issue. So this issue is still a design limitation and is not fixed.

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
4 participants