tc39
Supporing Records/Tuples and Maps
https://github.com/tc39/proposal-record-tuple/issues/275 brings up how groupBy could be supported on Tuples. Two open questions are:
- Should the groups be Array or Tuples?
- Should the output be an Object or a Record?
I think we can sidestep the first question for the time being (Array.p.groupBy groups into Arrays, and it wouldn't be weird for either choice to be chosen for Tuples).
But, I think we should address the second question now. We currently have Array.p.groupByToMap, and I don't think adding a new groupByToX is a great solution for new return types. I think it'll be a bit weird to have:
Array.p.groupByArray.p.groupByToMapArray.p.groupByToRecord(this would probably be bad, because how likely is it that the array is all primitives?)Tuple.p.groupByTuple.p.groupByToMapTuple.p.groupByToRecord
@denk0403 suggests in https://github.com/tc39/proposal-array-grouping/issues/30#issuecomment-1002542767 that we extend to new types by providing a static method on the constructor. Eg,
Array.p.groupBy(callback)returns a prototype-less ObjectTuple.p.groupBy(callback)returns a prototype-less ObjectMap.groupBy(iterable, callback)returns a MapRecord.groupBy(iterable, callback)returns a Record
If this is a change we want to make, now that it's stage 3 we'll need to notify implementors quickly to ensure they don't ship groupByToMap in the meantime. Hopefully this can be added to this month's agenda.
There are lots of methods in the language which produce string-keyed collections. Should we generally want to add record-producing versions of all of them?
I think no. We should instead make it easy to get a Record out of an object, so you can convert to the type you want if you want something non-default.
Map really is a different case, because you will very often want to group by something which is not a string. So it's not sufficient to convert after the fact.
I don't think we should hide the strictly more useful (if slightly less ergonomic) version of groupBy as a static method on Map. If we are to have only a single version on Array.prototype, the correct one to choose is groupByToMap. But I agree there are compelling reasons to want the slightly more ergonomic object-returning form to be on Array.prototype - so we should have both versions be there.
And unless we add a new kind of collection which has a different kind of coercion applied to its keys (which I very much hope we don't do), Map and Object are sufficient to cover all the kinds of grouping you might want to do. If you want a different type of collection, you can choose Map or Object as appropriate and then convert. So it's appropriate for those two, and only those two to be blessed on Array.prototype.
To put it more briefly: no, I don't think we should add Array.prototype.groupByToRecord or any equivalent, for the same reason I don't think we should add a Array.prototype.groupByToWeakMap. It is not practical to provide methods which directly produce every type of interest. Rather, we should pick the appropriate type to return for each API and make it straightforward to convert between types.
I agree, Record.groupBy or any other Array.prototype.groupByToX feels redundant if there’s eventually an easy way to convert between Objects and Records or other collection types.
I personally don’t have any strong opinions about the other methods' return types, but to me, it makes sense for Tuple.prototype.groupBy to return an object (with string or symbol keys), and tuples for values. As mentioned in https://github.com/tc39/proposal-record-tuple/issues/275#issuecomment-994826077, I think there is somewhat of an expectation that groupBy returns "sublists". And as the comment also mentions, it makes it trivial to convert the result to a Record since every inner value is already a primitive.
Tuple's Map version of groupBy obviously can't provide that same level of convenience for conversion, therefore I don't think it matters as much whether it returns a Map<K,Array> or a Map<K,Tuple>. However, perhaps one thing to consider is this: if the Map version of the method for tuples returns a Map<K,Array>, then its implementation is identical to the version on arrays. And if so, maybe then it's cleaner to place the implementation more centrally as a static method on Map.
On the other hand, if it should return a Map<K,Tuple>, then having distinct Array.prototype.groupByToMap and Tuple.prototype.groupByToMap methods makes more sense.
Building on the Map.groupBy idea further though, the static method could somewhat mirror the way Array.from works. Specifically, it would have the following signature:
groupBy(arrayLike: Iterable | ArrayLike, groupfn: (value, index) => any, thisArg?: any)
where:
arrayLike - An array-like or iterable object to convert to an array.
groupFn - grouping function to call on every element of the array.
thisArg Optional - Value to use as this when executing groupFn.
And like Array.from, subclasses of Map can inherit the method and return return instances of the subclass, not Map.
We support Map, and the Record & Tuple proposal can include a static groupBy method for consistency if it makes sense.