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

tyoshino commented on this pull request.



> @@ -182,50 +187,39 @@ function WritableStreamAddWriteRequest(stream) {
   return promise;
 }
 
-function WritableStreamFinishPendingWrite(stream) {
-  assert(stream._pendingWriteRequest !== undefined);
-  stream._pendingWriteRequest._resolve(undefined);
-  stream._pendingWriteRequest = undefined;
+function WritableStreamFinishInflightWrite(stream) {

OK. Done!

> @@ -109,6 +117,15 @@ function IsWritableStreamLocked(stream) {
   return true;
 }
 
+function WritableStreamIsReadyForWrites(stream) {

Checked this naming again. It looks not appropriate to me now.

This method is telling whether the ready promise is pending or not. Not indicating readiness. Actually, this includes `stream._backpressure === true` in the condition.

> @@ -533,6 +541,24 @@ function IsWritableStreamDefaultWriter(x) {
   return true;
 }
 
+function WritableStreamDefaultWriterEnsureReadyPromiseRejectedWith(stream, error, isPending) {

Yes, computing it early is the point of this pattern. Since the logic maps the current values of the several variables into the next value for the variable, calculating the next value in the middle of the sequence of codes would lead to take post-update value from some of the variables mistakenly.

But given that it's getting complicated by this pattern, I gave another try and picked a balance with which we can have readable code with a bit amount of duplicated code. PTAL

> @@ -122,22 +139,14 @@ function WritableStreamAbort(stream, reason) {
     return Promise.reject(error);
   }
 
-  assert(state === 'writable' || state === 'closing', 'state must be writable or closing');
+  assert(state === 'writable', 'state must be writable or closing');

Updated and moved above the _pendingAbortRequest checking line.

-- 
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-25432124