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

Tuple.prototype.groupBy #275

Open
acutmore opened this issue Dec 15, 2021 · 13 comments
Open

Tuple.prototype.groupBy #275

acutmore opened this issue Dec 15, 2021 · 13 comments

Comments

@acutmore
Copy link
Collaborator

Array.prototype.groupBy getting stage 3 made me think, I wonder what Tuple.prototype.groupBy would do 🤔

  1. return an object of arrays
  2. return a record of tuples
  3. return an object of tuples
@nicolo-ribaudo
Copy link
Member

The lists should be tuples, because they are "sublists" of the original tuple: tup.groupBy(computeKey).myKey is tup.filter(v => computeKey(v) === "myKey").

I don't have an opinion regarding the external wrapper, but if we choose to produce an object it will be easy to convert it to a record because it only contains primitives: Record(tup.groupBy(computeKey)).

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Dec 15, 2021

Well, since Tuple#groupByToMap will return a map probably Tuple#groupBy should return an object.

cc @jridgewell, if you have any opinion about this.

@ljharb
Copy link
Member

ljharb commented Dec 15, 2021

X.prototype.groupBy should return an object of X's, just like it does on Array, which means the values must be Tuples.

That said, it would be perfectly reasonable for it to return a Record of Tuples in this case, instead of an object of Tuples.

@jridgewell
Copy link
Member

X.prototype.groupBy should return an object of X's, just like it does on Array, which means the values must be Tuples.

I don't think must, but I think this is the right choice. Whether it's a record or an object, doesn't really matter to me.

@acutmore
Copy link
Collaborator Author

Thinking some more: Returning an object has the benefit of supporting grouping by symbol.

@ljharb
Copy link
Member

ljharb commented Dec 16, 2021

Why couldn’t you group by a symbol in a Record?

@nicolo-ribaudo
Copy link
Member

Records don't allow symbol keys (because they cannot be sorted).

@ljharb
Copy link
Member

ljharb commented Dec 16, 2021

Oof, right. In that case it should probably return an object.

@rickbutton
Copy link
Member

alternatively, Record.prototype.groupBy could return a record and just throw when you try to return a symbol from the callback, although kinda gross

@jridgewell
Copy link
Member

It was suggested in tc39/proposal-array-grouping#30 (comment) that we look at Map.groupBy(array, cb), and now that we're dealing with possible Record return values, that's is looking more promising. Instead of having a groupByMap that returns a Map instance, the constructor that the static groupBy is called on decides the output type.

So we could have:

  • Array.prototype.groupBy(cb), which returns an object
  • Tuple.prototype.groupBy(cb), which returns an object
  • Map.groupBy(arrayLike, cb), which returns a Map
  • Record.groupBy(arrayLike, cb), which returns a Record

@ljharb
Copy link
Member

ljharb commented Jan 11, 2022

I do like that.

@rricard
Copy link
Member

rricard commented Jul 8, 2022

We intend to have an opened draft PR but will only consider a merge once https://github.com/tc39/proposal-array-grouping goes to stage 4

@rricard
Copy link
Member

rricard commented Sep 27, 2022

PR is now open and will be merged conditionally if there is a progression to Stage 4 on proposal-array-grouping

@rricard rricard removed this from the stage 3 milestone Sep 27, 2022
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

No branches or pull requests

6 participants