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

domenic commented on this pull request.



> @@ -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');

assert message needs updating

> @@ -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) {

I guess we should treat "in flight" as two words, so inFlight/InFlight in code.

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

It seems nicer to have this recompute WritableStreamIsReadyForWrites each time. Is passing it in just an optimization, or is it important for correctness (e.g. computing it early in the function)?

> @@ -586,7 +611,7 @@ function WritableStreamDefaultWriterCloseWithErrorPropagation(writer) {
   assert(stream !== undefined);
 
   const state = stream._state;
-  if (state === 'closing' || state === 'closed') {
+  if ((state === 'writable' && stream._closeRequest !== undefined) || state === 'closed') {

Could this be simplified to just checking stream._closeRequest?

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