Promises based transform()

[tyoshino]

Branched from https://github.com/whatwg/streams/pull/180/files#r16783645

tyoshino

Do we want to also change transform() to use Promises to notify completion?

domenic

No, because transform() is more like ReadableStream: you can enqueue multiple chunks before calling done().

Although in theory, for both ReadableStream and for Transform, we could use a combination of enqueue + promises. I think it would be more awkward though? Unsure, might write up some sample code.

[calvinmetcalf]

there is precedence with node.js streams where values can either be passes to push (enqueue) or to the callback, in practice this works pretty well for the large percentage of the time where you are only transforming one thing.

[domenic]

@calvinmetcalf that is not what I am seeing with http://nodejs.org/docs/latest/api/stream.html#stream_transform_transform_chunk_encoding_callback ?

[calvinmetcalf]

That is how it works, though it isn't actually documented, (writing pull request for the documentation now)

[calvinmetcalf]

nodejs/node-v0.x-archive#8310

[domenic]

Consider the example at https://streams.spec.whatwg.org/#example-rs-pull, in particular:

    pull(enqueue, close, error) {
      const buffer = new ArrayBuffer(CHUNK_SIZE);

      fs.read(fd, buffer, 0, CHUNK_SIZE, position).then(bytesRead => {
        if (bytesRead === 0) {
          return fs.close(fd).then(close);
        } else {
          position += bytesRead;
          enqueue(buffer);
        }
      })
      .catch(error);
    },

with promises, this would become

    pull(enqueue, close) {
      const buffer = new ArrayBuffer(CHUNK_SIZE);

      return fs.read(fd, buffer, 0, CHUNK_SIZE, position).then(bytesRead => {
        if (bytesRead === 0) {
          return fs.close(fd).then(close);
        } else {
          position += bytesRead;
          enqueue(buffer);
        }
      });
    },

That is, it allows you to return a promise for error propagation. That's kind of nice. Not revolutionary. And I think it introduces a one-turn delay?

However, consider the case of a simple duplicater transform:

transform(chunk, enqueue, done, close, error) {
  enqueue(chunk);
  enqueue(chunk);
  done();
}

could become

transform(chunk, enqueue, close) {
  enqueue(chunk);
  enqueue(chunk);
}

which is a lot nicer. (But this time you definitely get a one-turn delay before we can call transform again.) Async transforms too:

transform(chunk, enqueue, done, close, error) {
  getAsyncData(chunk).then(d => { enqueue(d); done(); }, error);
}

becomes

transform(chunk, enqueue, close) {
  return getAsyncData(chunk).then(enqueue);
}

One thing that has me confused is that the current transform implementation doesn't pass close and error params like I do above. Perhaps because we expect the creator to just call transform.writable.close() and transform.writable.abort(er)? That seems lame. Maybe it was just an oversight.

---

I am actually somewhat convinced now that transform is more like writable than readable, contradicting my position from the OP. So it seems pretty clear that we want to be able to return promises from transform.

However, I'm not as sure about whether we want to use promises for readable-stream pull. I am leaning toward yes, since I think as long as we still use enqueue for enqueuing data, there is no extra one-turn delay. I need to analyze the algorithms a bit more to be sure.

This is still in my sprint for this week; hopefully I can figure out the modification to ReadableStream at least this weekend since we want to nail down ReadableStream ASAP.

[domenic]

#272 fixes this for readable streams. I will leave this open to track updating transform streams for a future sprint when I focus on those, but remove the "Week of 2015-01-19" milestone since the portion I wanted to accomplish last week is now finished.