fix(groupBy): return Partial Record

[jeysal]

The grouping classifier function may have never returned one of the possible values, so there is not necessarily an array for each group.

Note: The test is sort of copied from DefinitelyTyped.
I noticed you seem to use tsd expect* functions in the other test, but wanted to avoid putting a huge expectType of sorts.
Testing this way seems like a pretty good idea here, because it's user-focused ('this expression with the groupBy result is allowed, this one is not').
Feel free to amend the test if you don't like that part.

[Harris-Miller]

I 100% agree with your assertion about the return needing to be a Partial. I have some notes for your MR, lets get those done and we'll get this merged in!

[Harris-Miller]

The goal of the test are to tests the types, and not the actual function. So what is here does nothing, the way tsd works basically just ignores this file as is. We can use this as a base though. Add some test cases like this:

import { expectType } from 'tsd';
import { groupBy } from '../es';

const byGrade = groupBy((student: { score: number; name: string }) => {
  const score = student.score;
  return score < 65 ? 'F' : score < 70 ? 'D' : score < 80 ? 'C' : score < 90 ? 'B' : 'A';
});

expectType<(list: readonly { score: number; name: string }[]) => Partial<Record<'A' | 'B' | 'C' | 'D' | 'E' | 'F', { score: number; name: string }[]>>>(byGrade);
expectType<Partial<Record<'A' | 'B' | 'C' | 'D' | 'E' | 'F', { score: number; name: string }[]>>>(byGrade([]));

Check out some of the other existing test files for inspiration on how to cover all the possible cases

[jeysal]

It does work, actually! If you e.g. remove Partial from the return type, then grouped.C.length is allowed and thus the @ts-expect-error would throw a typecheck error. This is how DefinitelyTyped @types/ramda was tested.
In fact I'd argue it is a more resilient way of writing the tests because it focuses on what code is allowed and what code isn't, instead of expecting a long specific type that pretty much just repeats what the type definition says.

I can of course change it to use expectType assertions to be consistent with the other tests. But just a word of warning, this kind of tests will go red on pretty much any change to the types, and not just when a change was made that actually breaks something for the consumer of the types.

[jeysal]

Let me know if you really want me to add some expectTypes. My testing heart will be in slight pain extending the tests to be deliberately brittle, but I understand that codebase consistency also matters :D

[Harris-Miller]

It does work, actually! If you e.g. remove Partial from the return type, then grouped.C.length is allowed and thus the @ts-expect-error would throw a typecheck error

Oh yeah didn't think about how that would throw a typecheck error. Cool

[Harris-Miller]

Just convert what you have to use tsd and we'll be good

[jeysal]

added some tsd type assertions

[Harris-Miller]

This is good enough for me, let's merge it