tc39 / proposal-flatMap

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

Objections to stage 3 advancement #37

Closed domenic closed 6 years ago

domenic commented 6 years ago

I unfortunately was not able to attend the first day of TC39, but will be there the second and third days. I want to register that my concerns in https://github.com/tc39/proposal-flatMap/issues/34 are not resolved and that the spec text currently uses isConcatSpreadable, which I think is unacceptable regardless of how #34 is resolved. So, I object to this advancing to stage 3.

I'm not sure it's really appropriate for stage 2 in its current form, but it's not worth trying to make it go back based on process-lawyering over whether isConcatSpreadable is a "major semantic" or not. So let's just be sure to hold the proposal here until we've had a chance to exorcise isConcatSpreadable usage.

michaelficarra commented 6 years ago

@domenic What is your proposed alternative? In my presentation today, I explained to the satisfaction of everyone that was present that all considered alternatives were inferior.

Jessidhia commented 6 years ago

Flattening NodeList because it wants to pretend to be an array also has the undesirable property of losing the live-ness of the collection.

domenic commented 6 years ago

I don't think it's acceptable to use a protocol meant for the concat method in a method meant for flattening. My proposed alternative is a new protocol, but basically anything is better than reusing isConcatSpreadable.

domenic commented 6 years ago

Fixed by https://github.com/tc39/proposal-flatMap/commit/d73dcdc744608f76ab300ce115c9666de02b7a04

Jessidhia commented 6 years ago

🤔

Yeah, IsArray is probably better in that you can always deal with it by passing what you want to flatten through Array.from or [].concat() yourself inside the callback. If Array.from would misbehave on it (e.g. infinite iterator) then a flatten that processed iterators also would misbehave.