`toReversed()` is not reversible on sparse arrays

[Josh-Cena]

I can (with great regret) live with the fact that toSorted() eliminates empty slots (although I think symmetry with functionally similar APIs is much more charitable than symmetry by the time they were introduced—cf flatMap, findLast). However, the fact that toReversed() is not reversible bothers me.

[1, , 3].toReversed().toReversed(); // [1, undefined, 3]
[, , , ].toReversed(); // This is a symmetrical array; toReversed should be idempotent!

This makes its explanation unnecessarily harder, compared to just saying "every element is swapped", which automatically covers the empty slot case.

[zloirock]

This is the standard behavior of all ES6+ array methods - holes considered as undefined. For example,

[,,].includes(undefined) // => true

[Josh-Cena]

Right—but only those without functionally equivalent pre-ES6 counterparts. reverse() is reversible. flatMap() doesn't diverge from map() just because it's ES6.

[zloirock]

It was already discussed many times - for example, #8.

[Josh-Cena]

Maybe I should have mentioned I did read that thread, but I sent this issue specifically because I don't think "the general policy that all new array features since ES6 do not need to handle holes" is a justified argument, considering it breaks consistency with its mutating counterpart, breaks user expectation of what the method does, and makes documentation of this method unnecessarily harder. (For example, for reverse() I can just say "all elements are swapped", and it's automatically implied that empty slots are swapped, but this is not the case for toReversed(), which I have to add another paragraph that "empty slots become undefined". Occam's razor.) Literally all other odds are against this decision, such as drop-in replacement for arr.slice().reverse()—the only thing it's consistent with is [...arr].reverse(), but that uses the iterable protocol, not the array-like protocol.

[zloirock]

I mentioned that in #8 (comment)

[Josh-Cena]

I noticed that as well, which is why I'm surprised why you seem to have changed your stance here ^^ And I think that concern was never addressed either.

[zloirock]

I agree that it's better to handle holes as you propose, but it's not a strict position - those methods have much other deterioration compared to the originals - for example, killed subclassing support and killed %TypedArray%.prototype.toSpliced. Holes are not so the principal case for me.

[Josh-Cena]

I was just about to say that I can make the same case against using ArrayCreate rather than ArraySpeciesCreate. But it seems too hard to change TC39's mind at this point and I can't find many strong arguments in favor of @@species apart from (1) consistency (2) abstraction of implementation, so I decided to go with the low-hanging fruit first.

PS. I think removing %TypedArray%.prototype.toSpliced is fine if we are to have symmetry with the mutating counterpart. That I can somehow understand. But that only makes consistent handling of empty slots even more favorable.

[zloirock]

I don't think that "the mutating counterpart" is an argument since this proposal is based on R&T and .toSpliced added to immutable Tuple. Anyway, it's not related to holes -)

[ljharb]

I consider the prevailing belief, in the spec since ES2015 and in userland since long before, to be that whenever possible one should avoid ever having sparse arrays, and one should be overjoyed to encounter a method that fixes an array of its sparseness.

Every element is swapped. A hole’s element is undefined; its absence is a property of the original array, not of the element, and one shouldn’t expect that property to be copied over.

[Josh-Cena]

I agree that sparse arrays are suboptimal and should be avoided at all costs. I disagree that array methods should "autofix" them or conflate them with undefined. With far more existing methods special casing empty slots, developers are already made aware of the fact that empty slots are not the same as undefined—the array basically degenerates into an ad-hoc key-value sequence of non-consecutive non-negative integer keys. toReversed merely copies whatever keys the array already has over to a new array. (If you think about empty slots as an explicit state, then a DeletePropertyOrThrow is the only way to correctly "copy" them.) Also, I believe reverse sets a strong precedence for how it must behave, much stronger than any other prescriptive claims (which I still object, but not so strongly).