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

groupBy return either a Map or an OrderedMap: make the type more precise #1924

Merged
merged 6 commits into from Feb 1, 2023

Conversation

jdeniau
Copy link
Member

@jdeniau jdeniau commented Jan 9, 2023

No description provided.

@@ -4600,7 +4600,7 @@ declare namespace Immutable {
groupBy<G>(
grouper: (value: V, key: K, iter: this) => G,
context?: unknown
): /*Map*/ Seq.Keyed<G, /*this*/ Collection<K, V>>;
): Map<G, /*this*/ Collection<K, V>>;
Copy link
Member Author

@jdeniau jdeniau Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to override each class to be even more precise as

List<number>().groupBy(v => v); // is a Map<G, List<K, V>>
Set<number>().groupBy(v => v); // is an OrderedMap<G, Set<V, V>>
// etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to implement this, but got a lot of error:

Type instantiation is excessively deep and possibly infinite.

I did not manage to solve this, so I merge this as the current implementation is nearly right: we lose the fact that the Map is, sometimes, ordered, but we still gain all Map API.

@jdeniau jdeniau merged commit 0d2f2ba into main Feb 1, 2023
@jdeniau jdeniau deleted the ts-better-groupBy-type branch February 1, 2023 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant