Should push() wait for buffering, instead of a call to next()?

[jedwards1211]

I set out to make a simple lookahead async iterable, that fetches n upstream values in advance of the consuming for await loop.

Naively it seems obvious, just use a FixedBuffer(2) with a for await loop in the executor:

return new Repeater<T>(async (push, stop) => {
  for await (const value of asyncIterable) {
    await push(value)
  }
  await stop()
}, new FixedBuffer(2))

I assumed that push() would only wait if the buffer is full. But to my surprise, this wasn't accomplishing any lookahead at all, because push() waits for a call to next().

Instead I had to do this. I'm not sure if it's kosher?

const buffer = new FixedBuffer(2)
return new Repeater<T>(async (push, stop) => {
  for await (const value of asyncIterable) {
    if (buffer.full) await push(value)
    else push(value)
  }
  await stop()
}, buffer)

Regardless, it feels like this could be more ergonomic. WDYT?

[brainkim]

Thanks for the opening this issue! This appears to be a bug insofar as any push which is done against an open buffer should resolve to undefined. It appears that, currently, when there are pending calls to the iterator’s next() method, push calls will wait for the next next() rather than checking the buffer and resolving immediately.

The promise returned from push has always been a mildly confusing, insofar as it doesn’t resolve when the matching call to next() occurs, but the next one. This effectively means the first next call cannot be awaited by push(). This resembles the way yield expressions work in generator functions, but it always struck me as counter-intuitive. When you add a buffer things get more confusing, because you don’t want the push promise to block, but once the buffer is full it’s not immediately obvious to me which call to next() a push call should resolve to.

I think the fix is just an extra condition in the push() function which checks for a buffer but I’ll have to think about this.

Sorry for the delay and please let me know if you have any more thoughts.

[jedwards1211]

Well, I don't have too many more thoughts about this, because after I opened the issue I realized Web Streams are better for my use case than basic async iterables. And honestly I wonder if they're better than async iterables for most use cases? I still use repeaterjs for some graphql subscription use cases, but I think graphql subs would have been implemented on top of web streams if they had been established at the time.

[brainkim]

I’m in agreement about web streams. I still think the constructor with the double object arguments are a bit awkward, but you can’t beat built ins. I think they’re also async iterable these days, which wasn’t the case when I first wrote the library.