This bug is for implementing the Array.prototype.groupBy and Array.prototype.groupByMap proposal.

Depends on D132791

Depends on D132792

Depends on D133870

Depends on D133881

Backed out for causing failures on test_xrayToJS.xhtml

• backout: https://hg.mozilla.org/integration/autoland/rev/71853bc0eae7b7ba5599c6538625b73f74e882c7

• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&selectedTaskRun=b-l9ac_yTQ2t1xXBxgJ2vQ.0&revision=9473997f9d3545f9a101815cfa6ecfad49388946

• failure log: https://treeherder.mozilla.org/logviewer?job_id=363261724&repo=autoland&lineNumber=3896

[task 2022-01-06T17:39:44.805Z] 17:39:44     INFO - TEST-PASS | js/xpconnect/tests/chrome/test_xrayToJS.xhtml | Message correct: Error: Not allowed to define accessor property on [Object] or [Array] XrayWrapper 
[task 2022-01-06T17:39:44.806Z] 17:39:44     INFO - Buffered messages finished
[task 2022-01-06T17:39:44.810Z] 17:39:44     INFO - TEST-UNEXPECTED-FAIL | js/xpconnect/tests/chrome/test_xrayToJS.xhtml | A property on the Array prototype has changed! You need a security audit from an XPConnect peer - got "[\"at\", \"concat\", \"constructor\", \"copyWithin\", \"entries\", \"every\", \"fill\", \"filter\", \"find\", \"findIndex\", \"flat\", \"flatMap\", \"forEach\", \"groupBy\", \"groupByToMap\", \"includes\", \"indexOf\", \"join\", \"keys\", \"lastIndexOf\", \"length\", \"map\", \"pop\", \"push\", \"reduce\", \"reduceRight\", \"reverse\", \"shift\", \"slice\", \"some\", \"sort\", \"splice\", \"toLocaleString\", \"toSource\", \"toString\", \"unshift\", \"values\"]", expected "[\"at\", \"concat\", \"constructor\", \"copyWithin\", \"entries\", \"every\", \"fill\", \"filter\", \"find\", \"findIndex\", \"flat\", \"flatMap\", \"forEach\", \"includes\", \"indexOf\", \"join\", \"keys\", \"lastIndexOf\", \"length\", \"map\", \"pop\", \"push\", \"reduce\", \"reduceRight\", \"reverse\", \"shift\", \"slice\", \"some\", \"sort\", \"splice\", \"toLocaleString\", \"toSource\", \"toString\", \"unshift\", \"values\"]"
[task 2022-01-06T17:39:44.810Z] 17:39:44     INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:500:14
[task 2022-01-06T17:39:44.812Z] 17:39:44     INFO - testXray@chrome://mochitests/content/chrome/js/xpconnect/tests/chrome/test_xrayToJS.xhtml:440:7
[task 2022-01-06T17:39:44.813Z] 17:39:44     INFO - testArray@chrome://mochitests/content/chrome/js/xpconnect/tests/chrome/test_xrayToJS.xhtml:626:13
[task 2022-01-06T17:39:44.813Z] 17:39:44     INFO - go@chrome://mochitests/content/chrome/js/xpconnect/tests/chrome/test_xrayToJS.xhtml:163:5
[task 2022-01-06T17:39:44.814Z] 17:39:44     INFO - onload@chrome://mochitests/content/chrome/js/xpconnect/tests/chrome/test_xrayToJS.xhtml:1:1
[task 2022-01-06T17:39:44.816Z] 17:39:44     INFO - TEST-PASS | js/xpconnect/tests/chrome/test_xrayToJS.xhtml | A symbol-keyed property on the Array prototype has been changed! You need a security audit from an XPConnect peer 

https://hg.mozilla.org/mozilla-central/rev/8e29042ec08b
https://hg.mozilla.org/mozilla-central/rev/cacff53512a1
https://hg.mozilla.org/mozilla-central/rev/1de34f637810
https://hg.mozilla.org/mozilla-central/rev/b181e6c65058
https://hg.mozilla.org/mozilla-central/rev/e36262888093

Comment on attachment 9258477 [details]
Bug 1739648 - Add groupby and groupbytomap to xrayToJS only in nightly build

Revision D135604 was moved to bug 1749496. Setting attachment 9258477 [details] to obsolete.

Another regression. https://github.com/webcompat/web-bugs/issues/98458

FF98 nightly docs work for this can be tracked in https://github.com/mdn/content/issues/12579#issuecomment-1038626847

Note that I'm a little unclear why the groupByToMap() is useful. You can use object keys of course, as in the spec, https://github.com/tc39/proposal-array-grouping#proposal-array-grouping
However because Map uses same-value-zero equality. From this table you can see two objects that happen to have the same property names and values are not considered the same.

What that means is that if you want to use an object key you have to keep a hold of that forever, because otherwise you won't be able to get the group out of the map (since you can't recreate the same key). So why is this useful? I can see you might use it to create a group for undefined, which I guess could be handy.

The underlying reason this was included as part of the proposal according to the champion is that will be useful with records and tuples. In other words, you can create composite keys without defining an object outside of the function scope.

There are use cases for objects as keys in a map, and it is expected that you will hold a reference to it if you want to access it. You can see this in action here: https://searchfox.org/mozilla-central/rev/94d7c959115c03ea1e9406d6105b36cabe63775d/browser/components/extensions/parent/ext-devtools.js#197,217 -- Objects change over time, so transforming an object into a string representation will result in the key and the actual object getting out of sync -- leading to a loss of access to the values without some synchronization point. Otherwise, you need to do some form of mapping it to some identifier.

When you are using an object as a key in a map, it is because you are mapping related information to that specific object. The same applies to groupByToMap.

Thanks Yulia.

According to the spec on the callback(s):

If a thisArg parameter is provided, it will be used as the this value for each invocation of callbackfn. If it is not provided, undefined is used instead.

But I if not set, on FF nightly this actually appears to be the window object. The test code (from Will Bamberg here) is:

const inventory = [
  { name: 'asparagus', type: 'vegetables', quantity: 9 },
  { name: 'bananas',  type: 'fruit', quantity: 5 },
  { name: 'goat', type: 'meat', quantity: 23 },
  { name: 'cherries', type: 'fruit', quantity: 12 },
  { name: 'fish', type: 'meat', quantity: 22 }
];

let restock  = { restock: true };
const sufficient = { restock: false };
const result = inventory.groupByToMap( ({ quantity }) => {
  console.log(`it is: ${this}`)
  return quantity < 6 ? restock : sufficient
});
console.log(result.get(restock));

The log result for each item is:

it is: [object Window]

Can you please explain whether we're doing something wrong, or the implementation has a problem?

(In reply to Hamish Willee from comment #17)

Can you please explain whether we're doing something wrong, or the implementation has a problem?

1. Arrow functions have a lexical this, so they use the this value of the surrounding code instead of what's passed to the function.
2. If you change it to a non-arrow function, you then also have to use strict-mode ("use strict") or else undefined is turned into the global object when you use this.

Thanks Jan - a bit late, and FMI only:

I understand that you are saying a function has a particular this by default.
In the case of an arrow function it gets the this for whatever context it is called in, and in the case of another function in non-strict mode it is the global object. In strict mode a non-arrow function has no predefined this so unless you assign one it will be undefined.

The problem is that the spec says none of this - it just says If it is not provided, undefined` is used instead. It is usual for a spec to be so imprecise? I mean should it say something like "if it is not provided the value will follow the normal conventions for the type of function in which it is called"?

(In reply to Hamish Willee from comment #19)

The problem is that the spec says none of this - it just says If it is not provided, undefined` is used instead. It is usual for a spec to be so imprecise? I mean should it say something like "if it is not provided the value will follow the normal conventions for the type of function in which it is called"?

The groupBy spec invokes the callback with a certain this-value: either thisArg or undefined.

If the callback is a JS function, we then end up in the [[Call]] operation defined here: https://tc39.es/ecma262/#sec-ecmascript-function-objects-call-thisargument-argumentslist Step 5 there calls OrdinaryCallBindThis, defined here: https://tc39.es/ecma262/#sec-ordinarycallbindthis

In OrdinaryCallBindThis, steps 1-2 handle the arrow function case (lexical this). Steps 5-6 perform the undefined => globalObject conversion for non-strict mode.

So the spec is precise, but invoking a JS function involves some extra machinery that's defined by the [[Call]] operation.

Thank you Jan. That makes things completely clear!

Comment on attachment 9280297 [details]
Bug 1739648: Renaming to group and groupToMap r?yulia

Revision D148672 was moved to bug 1773471. Setting attachment 9280297 [details] to obsolete.

Comment on attachment 9280297 [details]
Bug 1739648: Renaming to group and groupToMap r?yulia

Revision D148672 was moved to bug 1773471. Setting attachment 9280297 [details] to obsolete.

https://hg.mozilla.org/mozilla-central/rev/8a9b5179c88c