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

Missing overload method on union types #60006

Closed
wheeOs opened this issue Sep 19, 2024 · 7 comments
Closed

Missing overload method on union types #60006

wheeOs opened this issue Sep 19, 2024 · 7 comments
Assignees
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.

Comments

@wheeOs
Copy link

wheeOs commented Sep 19, 2024

🔎 Search Terms

overlaod union

🕗 Version & Regression Information

  • This changed between versions ______ and _______
  • This changed in commit or PR _______
  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about overload
  • I was unable to test this on prior versions because _______

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.7.0-dev.20240919#code/FAMwrgdgxgLglgewgAgOYCcFgA4CECeAPACoB8AFAIbrqX4BcyAgjXSaQD7IBKAppQBMkAG3wtaRMgBpkAa14NklCPgCUjALKVshZfhni2ZUsgDewZMnS8YYdCmoSAdNYFgovQlp16DrSaSB5FRQUDK8wrwAtrwQMKrIALwm5haWyFBIAM4wcknIMPjYvAggcgpJiYnIAOTg0PBINcgA-Mjk8vhKWe0R0bEwjMQJyUoqquR9MXEJjFMDANqd3eX4pcjEALoA3GmWkbkYWNj5lKFOqDYdqrvpyHBl5ACERzgJ5neWZ1BOWVeyMleJ2qC02Nz2yAAvhCgU5sGAsgALSaRabxW7pay2exKUK7NKQmQQXgAd2Q3l0Kj8EnYFFU4OhwCAA

💻 Code

function groupBy<T>(array: Array<T>| ReadonlyArray<T>, key: any): Map<any, Array<T>> {
  return array.reduce<Map<any, Array<T>>>((acc, element) => {

    const k = typeof key === 'function' ? (key as (element: T) => any)(element) : element[key as keyof T];
    let group = acc.get(k);
    if (!group) {
      acc.set(k, group = []);
    }
    group.push(element);
    return acc;

  }, new Map<any, Array<T>>());
}

🙁 Actual behavior

getting the errors:

Expected 0 type arguments, but got 1.

Type 'T' is not assignable to type 'Map<any, T[]>'.
Expected 0 type arguments, but got 1.
Property 'get' does not exist on type 'T'.
Property 'set' does not exist on type 'T'.

🙂 Expected behavior

Allowing the use of the 3rd overload method reduce<U> of Array / ReadonlyArray

Additional information about the issue

No response

@jcalz
Copy link
Contributor

jcalz commented Sep 19, 2024

There doesn't seem to be a reason to write Array<T> | ReadonlyArray<T> since that's equivalent to ReadonlyArray<T> as shown in this playground link

But I kind of thought #53489 took care of these "union of array" things.

@wheeOs
Copy link
Author

wheeOs commented Sep 19, 2024

@jcalz

There doesn't seem to be a reason to write Array | ReadonlyArray since that's equivalent to ReadonlyArray

true that, thanks for pointing that out. I initially wrote the function for Array<T> , then I had to modify it for ReadonlyArray and it ended up like that, but apparently unnecessarily

@RyanCavanaugh
Copy link
Member

You're reporting this issue specifically?

function groupBy<T>(array: Array<T> | ReadonlyArray<T>) {
  array.reduce<any>((acc, element) => {
    throw new Error();
  });
}

@wheeOs
Copy link
Author

wheeOs commented Sep 19, 2024

@RyanCavanaugh yes the code sample can be reduced to that

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Sep 19, 2024
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 5.7.0 milestone Sep 19, 2024
@Andarist
Copy link
Contributor

Andarist commented Sep 19, 2024

But I kind of thought #53489 took care of these "union of array" things.

This was released in TS 5.2 and it's only about fallback signatures. It applies to situations when a signature can’t be found in a normal way but we can see that TS was able to find some signatures here even before that: TS 5.1 playground.

This case loses the relevant signatures in (somewhat complicated) getUnionSignatures. This function is able to compute some signatures (2 to be precise) in this case though so the referenced improvement from TS 5.2 doesn't even kick-in.

@weswigham
Copy link
Member

weswigham commented Sep 23, 2024

Yeah, the fallback logic is for unions of arrays over differing element types (to consider them close enough to a single array over the union of both element types) - this example is differing array types over the same element type. We end up saying we can't consider the reduce overload with a type argument as identical (rightly so) across the two array types because the 3rd argument of one callback is T[] while the other is readonly T[]. It's interesting here because we could just subtype reduce the signatures in this case and get a canonical signature set out pretty easily, which is what I've proposed we do opportunistically in #60036

@weswigham weswigham added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Sep 24, 2024
@weswigham
Copy link
Member

For now, we've chosen to leave this behavior as-is, unless we get some more frequent/compelling examples that require unifying multiple generic signatures to the subtype rather than dropping them, as we do today.

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 Fix Available A PR has been opened for this issue Needs Investigation This issue needs a team member to investigate its status.
Projects
None yet
6 participants