tc39 / proposal-flatMap

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

Set the array length explicitly #42

Closed ljharb closed 6 years ago

ljharb commented 6 years ago

Fixes #41

michaelficarra commented 6 years ago

This adds an observable Set operation for each recursive call of FlattenIntoArray. I don't think that's what we want. We only want one Set operation on the length property after the work has been completed.

ljharb commented 6 years ago

That'd be ideal; but 1) that's not how any of the existing examples in the spec do it, 2) if the TypeError exception gets thrown, the length wouldn't get set at all, 3) array subclasses might do a bunch of weird stuff when the length is set, and that'd be expected to happen when each item is added.

It also wouldn't be observable on an unmodified Array.prototype with a native array being the target, which I assume is how implementations optimize all the other spec operations.

michaelficarra commented 6 years ago

@ljharb That's not how, for example, Array.prototype.push works. It only sets the length once. Also, it's not just an unmodified Array.prototype, but also Object.prototype.

ljharb commented 6 years ago

Agreed on the unmodified prototypes for sure.

That's a fair point, given that, I'll set it once at the end instead.

ljharb commented 6 years ago

Should the Set be outside of FlattenIntoArray then, since it's recursive? iow, repeated in both flatten and flatMap?

michaelficarra commented 6 years ago

Yeah, I think so.

ljharb commented 6 years ago

Updated.