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<Tkey> has wrong return type #37

Closed
tuan-tu-tran opened this issue Jul 16, 2017 · 4 comments · Fixed by #38
Closed

GroupBy<Tkey> has wrong return type #37

tuan-tu-tran opened this issue Jul 16, 2017 · 4 comments · Fixed by #38

Comments

@tuan-tu-tran
Copy link
Contributor

Hi,

First, thanks for this, it's a really cool lib :-)

While I was using it, I noticed that in the typings, the return type for groupBy is odd:

linq/linq.d.ts

Line 121 in 27a8af6

groupBy<TKey>(keySelector: (element: T) => TKey): IEnumerable<IGrouping<TKey, any>>;

// truly, return type is IEnumerable<IGrouping<TKey, T>> but Visual Studio + TypeScript Compiler can't compile.
groupBy<TKey>(keySelector: (element: T) => TKey): IEnumerable<IGrouping<TKey, any>>;

Like the comment says, the true return type should be using T instead of any.
But I have the impression that the reason it invokes for not returning the right return type is moot.
I don't really know how this Visual Studio + TS compiler works but in the typescript project I'm working on, I just went and corrected it in my 'node_modules` folder directly and my editor started giving correct intellisense.

So I was wondering if you could just correct this.

If you want I can do a pull request.

Thanks!

@mihaifm
Copy link
Owner

mihaifm commented Jul 16, 2017

Yea, a pull req would be nice. Probably it didn't compile on older versions of typescript/visual studio, I'm not sure.

tuan-tu-tran added a commit to tuan-tu-tran/ng-theros that referenced this issue Jul 18, 2017
use a different overload of groupBy
whose typings are properly defined.

Until the upstream issue is fixed
mihaifm/linq#37
@mihaifm
Copy link
Owner

mihaifm commented Jul 18, 2017

thanks for this

@tuan-tu-tran
Copy link
Contributor Author

Yeah no problem. Thank you :)

Do you know when you plan on releasing this?

Cheers!

@mihaifm mihaifm reopened this Sep 4, 2017
@mihaifm
Copy link
Owner

mihaifm commented Sep 4, 2017

published a new version to npm, containing some other misc changes.
sorry for the huge delay

@mihaifm mihaifm closed this as completed Sep 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants