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

initial pass of Tuple group/groupToMap for proposal-array-grouping #317

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rickbutton
Copy link
Member

This PR represents a first pass addition of Tuple.prototype.group and Tuple.prototype.groupToMap, reflecting the addition of similar methods to the Array.prototype in https://github.com/tc39/proposal-array-grouping .

The array grouping proposal is currently at Stage 3, so we don't intend to merge this until Array Grouping reaches Stage 4, but want to prepare this in case it advanced ahead of Records and Tuples.

A few notes on this spec text:

  • This is almost entirely the same as the spec text for regular Array versions of these methods, which some verbiage changed to refer to Tuple.
  • This relies on the AO AddValueToKeyedGroup that the array grouping proposal implements, so if you are looking for that it is here: https://tc39.es/proposal-array-grouping/#sec-add-value-to-keyed-group
  • I removed the comments ahead of the algorithm steps regarding the mutation of the Array (Tuple) being traversed, as they don't apply here, since Tuples are immutable.
  • The return value of both of these new methods is an Object of Arrays. This choice was made so that a symbol can be used as a "grouping key" (because a Record cannot have a symbol key, see the discussion in this issue: Tuple.prototype.groupBy #275 (comment))

</emu-note>
<p>When the `groupToMap` method is called with one or two arguments, the following steps are taken:</p>
<emu-alg>
1. Let _O_ be ? ToObject(*this* value).
Copy link
Member

Choose a reason for hiding this comment

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

The other tuple methods we defined with explicit steps all use thisTupleValue(*this* value) instead of ToObject.

Copy link
Member Author

Choose a reason for hiding this comment

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

makes sense, I will adjust. I wasn't sure how much to deviate from the original Array versions of these methods.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you think I should instead use the same methodology behind other Array methods like .at and just specify them in terms of the original? Or manually specify the algo here as well.

Copy link
Member

Choose a reason for hiding this comment

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

manually is probably the better end goal regardless of how you do it in the proposal repo.

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 this pull request may close these issues.

3 participants