tc39 / proposal-flatMap

proposal for flatten and flatMap on arrays
https://tc39.github.io/proposal-flatMap
214 stars 19 forks source link

Why does flatten remove empty slots in arrays? #64

Closed Mouvedia closed 6 years ago

Mouvedia commented 6 years ago

Just curious. Id think keeping the same length for the arrays would be the safest and most logical; especially if you were dealing with indexes and unknown values.


var arr = [1, 2, [3, 4, [5, undefined, 7]]];
arr.flatten();

// [1, 2, 3, 4, [5, 7]]
// [1, 2, 3, 4, [5, undefined, 7]] <-- expected
ljharb commented 6 years ago

In your example, that’s what you’ll get i think? undefined isn’t a hole; but a hole means “no items”, so it’s treated the same as an empty array.

Mouvedia commented 6 years ago

That makes sense.

littledan commented 6 years ago

I'm very confused by this thread. It looks like the FlattenToArray algorithm was based on HasProperty algorithm (for detecting holes) and the IsArray algorithm (for seeing whether something should be treated as an Array). I thought this would lead to the "expected" output listed above. @michaelficarra , could you clarify?

ljharb commented 6 years ago

Yes - my understanding is that the “expected” output here is already what the proposal generates, since there’s no holes in the input. This thread is confusing because the OP has an incorrect (by my understanding) example for “currently”.

hax commented 5 years ago

I think this behavior is reasonable, but as https://github.com/tc39/proposal-flatMap/issues/62#issuecomment-373865035 imply, flat should replace [].concat.apply usage, and concat do not remove empty slots.

bakkot commented 5 years ago

I don't think this proposal's semantics need to precisely match those of [].concat.apply. It already differs in that this proposal switches off isArray rather than Symbol.isConcatSpreadable.

hax commented 5 years ago

@bakkot

Though I'm still not convinced why Symbol.isConcatSpreadable is bad (my impression is the champion just change the semantic arbitrary in one day when domenic show objection to stage 3, without writing any rationale and waiting feedback in the original issue --- I don't think it's a healthy way), at least no one really use it in real world, most js programmers even never heard about Symbol.isConcatSpreadable.

On the contrary, the behavior difference between old [].concat.apply pattern and current semantic is more realistic. As I said, I think remove empty slots is ok, but I hope you TC39 guys could give more evidence-based reasoning, for example, check what flatten in current libraries do, instead of just rely on your personal preference.

Thank you.