tc39 / proposal-async-iterator-helpers

Methods for working with async iterators in ECMAScript
https://tc39.es/proposal-async-iterator-helpers/
107 stars 3 forks source link

How will `flatMap` handle `undefined` returns? #17

Open benlesh opened 8 months ago

benlesh commented 8 months ago

I'm a little fuzzy on how this (or iterator helpers) will handle undefined returns. Array#flatMap will just add an undefined value to the output flattened array. I assume the same is true here?

const result = someAsyncIterator.flatMap(v => { });

for await (const value of result) {
  console.log(value); // just prints `undefined` over and over.
}
bakkot commented 8 months ago

It throws if the mapper returns any non-iterator/iterable value, including undefined. (As a separate special case, it also throws if the mapper returns a string.)

You can try it today in Chrome 122+: [0].values().flatMap(x => [1, 2]).toArray().

bakkot commented 8 months ago

(For async helpers, it will await the value first, and also allows async iterables. But otherwise it's the same, and since await undefined is not an iterator nor a sync or async iterable, it will throw.)

conartist6 commented 8 months ago

It's unfortunate that mapper returning a string will error out, i.e. that flatmap works on almost all iterables but not all of them

ljharb commented 8 months ago

I would consider that an intentional design decision; it's very harmful that strings are directly iterable and in new APIs we're generally requiring the iterable to be an object specifically to exclude strings.

bakkot commented 8 months ago

That's very much intentional. See discussion in https://github.com/tc39/proposal-iterator-helpers/issues/244, https://github.com/tc39/proposal-iterator-helpers/issues/229, etc.

Wanting to treat a string as iterable for purposes of flatMap is such a weird thing to do that it seems fine/good that you have to explicitly signal that you really did intend to do so (e.g. by returning string[Symbol.iterator]()) rather than it happening by default.

In any case, that's off-topic for this issue.

conartist6 commented 8 months ago

Oh I'm sure it's intentional, yeah. I understand why too. Sorry, I swear I'm not trying to stir up controversy everywhere I go today.