Skip to content

Overloaded arrow function requires any param, function syntax accepts union #37824

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
cayhorstmann opened this issue Apr 7, 2020 · 4 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@cayhorstmann
Copy link

cayhorstmann commented Apr 7, 2020

TypeScript Version: 3.8.3

Search Terms:
Overloading, arrow functions, type union, any
Code

const remove: {
  (str: string, c: string): string
  (str: string[], c: string): string[]
} = (str: string | string[], c: string) =>{
  if (Array.isArray(str))
    return str.map(s => s.replace(c, ''))
  else
    return str.replace(c, '')
}

Expected behavior:

Either the function compiles, or the version with function also fails to compile

function remove2(str: string, c: string): string
function remove2(str: string[], c: string): string[]
function remove2(str: string | string[], c: string) {
  if (Array.isArray(str))
    return str.map(s => s.replace(c, ''))
  else
    return str.replace(c, '')
}

Actual behavior:

The following error occurs with the arrow version:

  Type 'string | string[]' is not assignable to type 'string'.
  Type 'string[]' is not assignable to type 'string'.(2322)

I must change the parameter of the implementation from string | string[] to any for the code to compile.

However, the version with function compiles.

Playground Link:

Playground link

Related Issues:

Note that this is subtly different from #33482 where the return type of the implementing function was questioned. Here, no return type is specified. The issue is the parameter type.

I understand the error message. Instead of looking at the behavior of the function, it is simply stated that (str: string | string[], c: string) => string | string[] is not compatible with the LHS type. That is undispably the case.

However, the function overload traces the function behavior, finding that string arguments lead to string return values and string[] arguments to string[] return values. The same could be done here: when the LHS is typed as an overload and the RHS is a literal arrow function, check whether the particular instance (and not any instance with that type) is assignable.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 9, 2020

The problem isn't actually incompatibility in the parameter - it's the return type. You can check by trying this out.

  const remove: {
  (str: string, c: string): string
  (str: string[], c: string): string[]
- } = (str: string | string[], c: string) =>{
+ } = (str: string | string[], c: string): any =>{
  if (Array.isArray(str))
      return str.map(s => s.replace(c, ''))
  else
      return str.replace(c, '')
  }

It's effectively impossible for most implementations to satisfy every overload's return type using naive assignability rules. You have to satisfy every overload's return type, meaning the implementation's return type needs to be a subtype of (e.g. an intersection of) every overload's return type.

That really doesn't make much sense in practice because the intent is that the implementation needs to do work conditionally, not return one value whose type that satisfies every possible case. So in #6075 we added a special bivariance check between return types of overloads and implementations. It's an unsound laxness, but it allowed people to model these cases more easily.

We didn't add this check for the general case of checking assignability between a function to a type with overloads because it wasn't clear if the intent was going to be the same. Can I ask how this example came up? Is it just a stylistic preference?

@cayhorstmann
Copy link
Author

What you suggest is not quite the same thing. If I make a mistake in the implementation, the function version catches it and the arrow version (modified as per your suggestion) does not.

function remove2(str: string, c: string): string // Error: This overload signature is not compatible with its implementation signature.
function remove2(str: string[], c: string): string[]
function remove2(str: string | string[], c: string) {
  if (Array.isArray(str))
    return str.map(s => s.replace(c, ''))
  else
    return [str.replace(c, '')] 
}

const remove: {
  (str: string, c: string): string
  (str: string[], c: string): string[]
} = (str: string | string[], c: string) : any =>{
  if (Array.isArray(str))
    return str.map(s => s.replace(c, ''))
  else
    return [str.replace(c, '')] // No error
}

What exactly is happening in the function version? Is the typechecker following the string and string[] branches? If so, couldn't it do the same thing in the arrow function case?

Or is there another way of doing this with an arrow function? I tried typing remove as a union type (str: string, c: string) => string | (str: string[], c: string) => string[]. That didn't work at all.

@cayhorstmann
Copy link
Author

Since you mention #6075, I had another look. Consider

function remove2(str: string, c: string): string
function remove2(str: string[], c: string): string[]
function remove2(str: string | string[], c: string) : string | string[] {
  if (Array.isArray(str))
    return str.map(s => s.replace(c, ''))
  else
    return str.replace(c, '')
}

That works. And as #33482 complains, it doesn't work with arrow functions.

But actually, specifying the return type doesn't seem to be a good idea in the function case. Introducing the same error does not cause TypeScript to notice anything amiss.

function remove2(str: string, c: string): string
function remove2(str: string[], c: string): string[]
function remove2(str: string | string[], c: string) : string | string[] {
  if (Array.isArray(str))
    return str.map(s => s.replace(c, ''))
  else
    return [str.replace(c, '')]
}
let result = remove2('Fred', 'e') 
console.log({result}) // It's an array: { "result": [ "Frd" ] } 
let greeting: typeof result = 'Hello' // But TypeScript thinks it's a string

I naïvely assumed that TypeScript did a flow analysis of the function code, but now I don't know what it does.

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Apr 13, 2020
@typescript-bot
Copy link
Collaborator

This issue has been marked as 'Question' and has seen no recent activity. It has been automatically closed for house-keeping purposes. If you're still waiting on a response, questions are usually better suited to stackoverflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

4 participants