Re: [whatwg/streams] Factor out condition to check pending operation (#672)

domenic commented on this pull request.



>    stream._state = 'errored';
-  stream._storedError = new TypeError('Abort requested but closed successfully');
+  stream._storedError = error;
+
+  // TODO(tyoshino): No need to reject readyPromise?

What's the plan for this TODO?

> @@ -407,23 +426,33 @@ class WritableStreamDefaultWriter {
 
     const state = stream._state;
 
-    if (state === 'writable' || state === 'closing') {
+    if (state === 'writable') {
+      if (stream._pendingAbortRequest !== undefined) {
+        // TODO(tyoshino): Test this

Status on this TODO?

>        defaultWriterClosedPromiseInitializeAsResolved(this);
     } else {
       assert(state === 'errored', 'state must be errored');
 
-      defaultWriterClosedPromiseInitializeAsRejected(this, stream._storedError);
+      const storedError = stream._storedError;
+      // TODO(tyoshino): Get consensus on this behavior i.e. readyPromise rejection reason may differ between
+      // release.

This seems good to me as-is, so I would delete this TODO

>  
   WritableStreamDefaultControllerAdvanceQueueIfNeeded(controller);
 }
 
+function WritableStreamDefaultControllerStart(controller) {
+  const startResult = InvokeOrNoop(controller._underlyingSink, 'start', [controller]);
+  Promise.resolve(startResult).then(
+    () => {
+      controller._started = true;
+      // TODO: Test this.

Another test TODO

> @@ -2680,25 +2693,19 @@ WritableStream(<var>underlyingSink</var> = {}, { <var>size</var>, <var>highWater
   behavior will be the same as a {{CountQueuingStrategy}} with a <a>high water mark</a> of 1.
 </div>

It would be good to add the note about how the inFlightX slots are mutually exclusive with the X slots here (as a `<p class="note">`).

>  <emu-alg>
   1. Set *this*.[[state]] to `"writable"`.
-  1. Set *this*.[[storedError]], *this*.[[writer]], *this*.[[writableStreamController]], *this*.[[pendingAbortRequest]],
-     *this*.[[pendingCloseRequest]], and *this*.[[pendingWriteRequest]] to *undefined*.
+  1. Set *this*.[[storedError]], *this*.[[writer]], *this*.[[writableStreamController]],
+     *this*.[[inFlightWriteRequest]], *this*.[[closeRequest]], *this*.[[inFlightCloseRequest]],

missing "and"

>    1. Let _controller_ be _stream_.[[writableStreamController]].
   1. Assert: _controller_ is not *undefined*.
-  1. Let _readyPromiseIsPending_ be *false*.
-  1. If _state_ is `"writable"` and !
-     WritableStreamDefaultControllerGetBackpressure(_stream_.[[writableStreamController]]) is *true*, set
-     _readyPromiseIsPending_ to *true*.
-  1. If _controller_.[[writing]] is *false* and _controller_.[[inClose]] is *false*,
-    1. If _stream_.[[writer]] is not *undefined*, perform !
-       WritableStreamDefaultWriterEnsureReadyPromiseRejectedWith(_stream_.[[writer]], _error_, _readyPromiseIsPending_).
+  1. Let _writer_ be _stream_.[[writer]].
+  1. If _writer_ is not *undefined*,
+    1. If ! WritableStreamCloseQueuedOrInFlight(_stream_) is *false* and _stream_.[[backpressure]] is *true*,
+       <a>reject</a> _writer_.[[readyPromise]] with _error_.
+    1. Otherwise, set _writer_.[[readypromise]] to <a>a promise rejected with</a> _error_.

uppercase "p" in readyPromise

>      1. Return.
   1. <a>Resolve</a> _stream_.[[pendingAbortRequest]] with *undefined*.
   1. Set _stream_.[[pendingAbortRequest]] to *undefined*.
+  1. Let _error_ to a new *TypeError* indicating that the stream was closed successfully after abort request.

Hmm I'm not sure on the error phrasing here. Could you describe the situation in a bit more detail?

>    1. Perform ! WritableStreamDefaultControllerAdvanceQueueIfNeeded(_controller_).
 </emu-alg>
 
+<h4 id="writable-stream-default-controller-start" aoid="WritableStreamDefaultControllerStart" nothrow>

This needs to be "throws" since step 1 could throw.

-- 
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/672#pullrequestreview-25860228