Re: [whatwg/streams] make TransformStream respect backpressure (#550)

ricea commented on this pull request.

This is great. It's much easier to understand than it used to be.

>  
-  transformStream._resolveWrite(undefined);
-  transformStream._resolveWrite = undefined;
+  let readyPromise;

I think this would be easier to understand without the `readyPromise` variable. How about

```javascript
if (transformStream._readableBackpressure === false) {
    return Promise.resolve();
}
return new Promise(resolve => {
  transformStream._readyPromise_resolve = resolve;
});
```

>    const controller = transformStream._transformStreamController;
   const transformPromise = PromiseInvokeOrNoop(transformStream._transformer,
                              'transform', [chunk, controller]);
 
-  transformPromise.then(() => TransformStreamResolveWrite(transformStream),
-                     e => TransformStreamErrorIfNeeded(transformStream, e));
+  return transformPromise.then(() => {
+    transformStream._transforming = false;
+  }).then(() => TransformStreamReadyPromise(transformStream),
+          e => TransformStreamErrorIfNeeded(transformStream, e));

I don't think this will be easy to read when translated into an algorithm in the spec.

How about

```javascript
return transformPromise.then(
    () => {
      transformStream._transforming = false;
      TransformStreamReadyPromise(transformStream);
    },
    e => TransformStreamErrorIfNeeded(transformStream, e));
```

> @@ -186,31 +172,18 @@ class TransformStreamSink {
 
     transformStream._writableController = c;
 
-    return this._startPromise;
+    // delay all sink.write() calls until there is no longer backpressure.
+    return this._startPromise.then(() => {
+      return TransformStreamReadyPromise(transformStream);

Don't need a block here. Can just write

```javascript
return this._startPromise.then(() => TransformStreamReadyPromise(transformStream));
```


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/550#pullrequestreview-5438723

Received on Monday, 24 October 2016 13:03:48 UTC