nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.33k stars 29.47k forks source link

stream: alternative async generator transform #39279

Open ronag opened 3 years ago

ronag commented 3 years ago

Just an idea for discussion:

Currently we support creating transform streams from async generators:

async function* toUpper(source) {
  for await (const chunk of source) {
    yield source.toUpperCase();
  }
}

However, after reading through https://github.com/tc39/proposal-function.sent I realize there is an alternative possible syntax:

async function* toUpper() {
  while (!this.signal.aborted) {
    yield this.sent.toUpperCase();
  }
}

Which has an advantage of avoiding the extra parameter required in toUpper and could unify the signature of readable and transform generators.

I'm not sure about whether or not this is a good idea. Mostly wanted to raise awareness of the possibility and see what others think.

I believe we could probably achieve a similar syntax pre this proposal by replacing function.sent with this.sent for use with pipeline/compose as a compat. It would have the same value as function.sent for forward compatibility.

Example how this could be implemented:

async function* processor() {
  while (true) {
    yield await process(this.sent, { signal: this.signal });
  }
}

async function* src() {
  yield 'hello'
  yield 'world'
}

async function* pump(src, gen) {
  const ac = new AbortController()
  const context = { sent: null, signal: ac.signal }
  const dst = gen.call(context)
  const iterator = dst[Symbol.asyncIterator]()
  try {
    for await (const chunk of src) {
      context.sent = chunk
      const { done, value } = await iterator.next()
      if (done) {
        return
      }
      yield value
    }
  } finally {
    ac.abort()
    iterator.return()
  }
}

for await (const chunk of pump(src(), processor)) {
  console.log(chunk)
}
ronag commented 3 years ago

@benjamingr I'm not sure we even need #39067 if we do it like this?

ronag commented 3 years ago

Some limitations/problems with this approach:

benjamingr commented 3 years ago

I actually like passing signal and the source stream explicitly I think it's more obvious and makes it clear they need to be used.

I think using this is a bit quirky here

benjamingr commented 3 years ago

Also, what ever happened to function.sent? @ljharb is it just blocked on people actually caring/doing the spec work?

ronag commented 3 years ago

I wonder if this has any performance benefits.

ljharb commented 3 years ago

@benjamingr yes, the previous champion no longer participates in TC39, and it was about to be made inactive, but then another delegate took it up, but nothing's been done with it.