tc39 / proposal-iterator-sequencing

a TC39 proposal to create iterators by sequencing existing iterators
https://tc39.es/proposal-iterator-sequencing
31 stars 1 forks source link

Iterator.concat instead of variadic Iterator.from #1

Open bakkot opened 6 months ago

bakkot commented 6 months ago

Variadic Iterator.from is weird in a couple of ways:

So I would prefer not to do this. I'd prefer instead having a static (or prototype) concat method, which works the same way as the variadic case of from currently in this proposal.

graphemecluster commented 5 months ago

Taking consistency into consideration, Iterator.from(x, y) should instead behave like Iterator.from(x).map(y) (I can’t find anyone talking about it even with a thorough search – am I missing something?)

michaelficarra commented 5 months ago

@graphemecluster The second parameter of Array.from is there to avoid having to iterate the whole array after iterating the first parameter. There's no such downside with Iterator.from(a).map(b), so matching the signature of Array.from would be pointless.

zloirock commented 5 months ago

It's an inconsistency that will make JS harder to learn and memorize.

There's no such downside with Iterator.from(a).map(b)

If a does not have %IteratorPrototype% in the prototype chain (userland iterator), it's 2 wrappers instead of 1.

ljharb commented 5 months ago

Iterator.from always produced a wrapped iterator, so the return will always have .map on it.

zloirock commented 5 months ago

Iterator.from always produced a wrapped iterator

Nope.

image

ljharb commented 5 months ago

Thanks, I'll rephrase: Iterator.from always produces an iterator that has .map on it (in a world where iterator helpers are shipped) - or at least, that was the entire point of it.

zloirock commented 5 months ago

I'll rephrase

I have no idea how it's related to this case.


In

Iterator.from({
  next: () => ({ done: Math.random() > .9, value: Math.random() })
}).map(it => it * 10 | 0).toArray();

Iterator.from produces an intermediate iterator (with WrapForValidIteratorPrototype), .map produces a second wrapper (with IteratorHelperPrototype). Like with an intermediate array in Array.from().map(), a little cheaper - but anyway it's overhead with extra proxied .next calling. A mapper as the second Iterator.from argument can reduce this overhead (a minor advantage) and make it a little more consistent with Array.from (a more significant advantage).

It's not something I can't live without - it's just an example that consistency is better.

bakkot commented 5 months ago

@ljharb said he would want any method named concat to share Array.prototype.concat's behavior of allowing both containers and items which are not containers, with the containers being spread and the non-containers used as-is.

@michaelficarra said he wasn't interested in that behavior. Combined, that rules out the name concat.

So, instead, how about a static variadic Iterator.append? I think that would be quite clear: Iterator.append(foos, bars).

ljharb commented 5 months ago

Seems perfectly reasonable to me!

bergus commented 5 months ago

@ljharb said he would want any method named concat to share Array.prototype.concat's behavior of allowing both containers and items which are not containers

Why is that? I believe more people are confused by this behaviour than pleased by it, and would gladly drop it (if it wasn't for backwards compatibility). Having to check for isConcatSpreadable every time can't be efficient either, can it? Also String.prototype.concat already doesn't follow this rule, unless you treat strings as containers of characters and distinguish them from single-character strings. And I don't see why this convention would need to hold for static methods?

how about a static variadic Iterator.append?

Hm, I think that would confuse Pythonistas where list.append(…) is the single-element-push method. In my ears, append is at best a binary instance method, like foos.append(bars), not a static variadic function. Maybe that's my Haskell upbringing, where concat :: [[a]] -> [a] and (m)append :: [a] -> [a] -> [a].

I'd definitely prefer Iterator.concat(...iterables). If that's out of question already, maybe Iterator.concatenate(...iterables)?

ljharb commented 5 months ago

Not because of isConcatSpreadable, but because it’s very very idiomatic to do [].concat(maybeArray || []).

Nobody ever uses string concat (other than Babel’s template literal transpile).

Anything that’s similar to concatenation implies the same behavior as array concat; “concatenation” is this the same as “concat” in this regard,

ljharb commented 4 days ago

(i note the readme still says the chosen solution is called "concat"; my constraint remains that if it's called "concat" it must accept both iterables and non-iterables; alternatively a new name can be chosen)

michaelficarra commented 4 days ago

Yeah at the June meeting I presented some names we may move forward with if concat can't work.

Iterator Sequencing for Stage 2

ljharb commented 4 days ago

I'd vote to avoid concatenate or cat, due to possible confusion (cat is a shell command).

bakkot commented 4 days ago

My vote is still for concat, or failing that for append.

I am somewhat reluctant to give up on the obvious name based only on a single person's dislike of that name. I see the inconsistency you're pointing out but since it's already different by virtue of being static rather than prototype, and since you'll immediately get an error if you try to use it assuming it has the same behavior as [].concat, I don't think that inconsistency is enough reason to use a different name.