Re: [whatwg/streams] Update TransformStream API & misc. fixups (#519)

tyoshino commented on this pull request.



> -    } else {
-      try {
-        transformStream._transformer.flush(
-            transformStream._enqueueFunction,
-            transformStream._closeFunction,
-            transformStream._errorFunction);
-      } catch (e) {
-        if (transformStream._errored === false) {
-          TransformStreamErrorInternal(transformStream, e);
-          throw e;
-        }
-      }
-    }
+    const flushPromise = PromiseInvokeOrNoop(transformStream._transformer, 'flush', [transformStream._controller]);
+    // Return a promise that is fulfilled with undefined on success.
+    return flushPromise.then(() => TransformStreamCloseReadableInternal(transformStream),

I understand your point. But we could also say that close() becoming disallowed in flush() would make the API a little complicated. Regarding the priority concern, I think we should make the transform stream accept the signal which arrived to it earliest. That said, this kind of rule also complicates the API.

Another question I'm not yet fully sure about is whether the resolution of flushPromise which you've changed to be propagated to writer.close() and readable's final state should always match or not. If close() / error() are allowed inside flush(), we can e.g. close() readable side while making writer.close() rejected. My gut feeling is basically they should match each other, but it seems worth being discussed a bit.

-- 
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/519