Spec should not handle sparse arrays

[mhofman]

The polyfill uses the spread operator to clone which means that holes are not skipped like in the proposed spec.

[ljharb]

Should holes be skipped? I think all array methods ES6+ have explicitly not skipped holes.

There's a consistency argument to be made, though, that skip behavior of the "d" version should match the "non-d" version.

[zloirock]

This polyfill does not handle subclassing but seems it's just something for showing basic functionality.

It could be fixed just by replacing spread to .slice.

[zloirock]

@ljharb I'm for consistency.

[ljharb]

Consistency with ES6 and new array methods, or consistency with the non-d version of any given d method?

[zloirock]

With non-d, sure - it's non logically to add unnecessary inconsistency.

[ljharb]

There's inconsistency here regardless - it's inevitable. All we're doing is choosing which consistency/inconsistency we prefer.

[zloirock]

Those methods at first are versions of old methods so they should follow the old logic.

[mhofman]

I went by fidelity to the proposed spec which checks for property existence, and consistency with corresponding mutating array methods which do skip or otherwise respect holes.

[zloirock]

Even don't look to the consistency with non-d methods - I think that it's not a case of array iteration where ES6+ methods should not skip holes, it's the logic of complete shallow array cloning - the array should be the same as the original - with the same holes if they are presented - here no ES6+ equals - only old methods like .slice - and they logic here works perfectly.

[rricard]

So I agree holes should not be skipped, I will note that for the next spec draft. (please note that the spec is definitely a work in progress at stage 0)

[zloirock]

@rricard it will break consistency with non-d methods:

[1,,,,2].reverse().forEach(console.log); // => 2, 1
[1,,,,2].reversed().forEach(console.log); // => 2, undefined, undefined, undefined, 1

[rricard]

Ok, I will add the question tag because it is not obvious which one to choose.

[rricard]

After discussing in the R&T monthly call, we are deciding to break consistency with non-'ed methods and will convert holes to undefined. This is in line with the general policy that all new array features since ES6 do not need to handle holes.

We are aware, this doesn't make the feature a drop-in replacement for arr.slice().reverse() for instance as [,,1].slice().reverse() will give [1,,] while [,,1].reversed() will give [1, undefined, undefined].

We can discuss it further if you feel it should be discussed but please know we got a pretty strong signal against holes.

This still needs to be properly reflected in the spec and will be as we work on fixing #7.

[rricard]

seems resolved across spec and polyfill