`Array.prototype.groupBy` and `Array.prototype.groupByToMap` just advanced to TC39 stage 3 Proposal: <https://github.com/tc39/proposal-array-find-from-last> Spec: <https://tc39.es/proposal-array-find-from-last/index.html> ``` const array = [1, 2, 3, 4, 5]; array.groupBy(n => n % 2 ? "odd" : "even") // { odd: [1, 3, 5], even: [2, 4] } array.groupByToMap(n => n % 2 ? "odd" : "even") // Map [["odd", [1, 3, 5]], ["even", [2, 4]] ```
Created attachment 447253 [details] [Patch] WIP I am putting this up so that I can let EWS take a stab at it while I write some specific tests :)
Created attachment 447289 [details] Patch
Comment on attachment 447289 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447289&action=review r=me > Source/JavaScriptCore/builtins/ArrayPrototype.js:192 > + var key = callback.@call(thisArg, value, i, array); Can you include a test, returning "__proto__" for key string? Like, Array.prototype.ok = 42; // enumerable. var result = [......].groupByToMap((value, index, array) { return "__proto__"; }); result.has("ok"); // false
Created attachment 447312 [details] Patch
Comment on attachment 447289 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447289&action=review > Source/JavaScriptCore/builtins/ArrayPrototype.js:193 > + var group = (groups[key] ??= []); You've probably already noticed this, but I comment either way: ToPropertyKey that is performed by `groups[key]` is not in the spec (unlike Array.prototype.groupBy), and rightfully so because Maps can have keys beyond strings & symbols. Considering step 6.d, I would suggest `var groups = new @Map` since it's how Maps sanitise keys as well. Also, it would be nice to ensure we have test cases with throwing toString() / valueOf() and -0 / 0 keys. The spec I'm looking at is https://tc39.es/proposal-array-grouping.
Created attachment 447320 [details] Patch
Comment on attachment 447320 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447320&action=review > Source/JavaScriptCore/builtins/ArrayPrototype.js:200 > + if (key === -0) This is extra before Map.prototype.set, which performs -0 => +0 coercion at step 5 (https://tc39.es/ecma262/#sec-map.prototype.set). I'm pretty sure we can just pass `key` directly to Map's @get / @set.
Created attachment 447321 [details] Patch
Comment on attachment 447321 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447321&action=review > Source/JavaScriptCore/builtins/ArrayPrototype.js:173 > + var group = groups[key]; Aren't we performing ToPropertyKey twice here (the second time @ L176)? A test with counter would be hugely appreciated.
Created attachment 447323 [details] Patch
Created attachment 447330 [details] Patch
Committed r287136 (245320@main): <https://commits.webkit.org/245320@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447330 [details].
<rdar://problem/86576923>