Re: [whatwg/streams] Support teeing readable byte streams (#1114)

@MattiasBuelens commented on this pull request.



> @@ -3205,7 +3442,7 @@ The following abstract operations support the implementation of the
  1. Let |firstDescriptor| be |controller|.[=ReadableByteStreamController/[[pendingPullIntos]]=][0].
  1. If |firstDescriptor|'s [=pull-into descriptor/byte offset=] + |firstDescriptor|' [=pull-into
     descriptor/bytes filled=] is not |view|.\[[ByteOffset]], throw a {{RangeError}} exception.
- 1. If |firstDescriptor|'s [=pull-into descriptor/byte length=] is not |view|.\[[ByteLength]], throw
+ 1. If |firstDescriptor|'s [=pull-into descriptor/byte length=] < |view|.\[[ByteLength]], throw

I believe this was a missing feature of `byobRequest.respondWithNewView(view)`. You could already call `byobRequest.respondWith(bytesWritten)` with `0 <= bytesWritten <= byobRequest.view.byteLength`, but for some reason `.respondWithNewView(view)` **insisted** that you fill the entire view. Now, you can pass any view whose size is less than or equal to the original view's size. 🙂

Or would you prefer fixing this in a separate PR?

> +      reader = AcquireReadableStreamDefaultReader(stream);
+      forwardReaderError(reader);
+    }
+
+    const readRequest = {
+      chunkSteps: chunk => {
+        // This needs to be delayed a microtask because it takes at least a microtask to detect errors (using
+        // reader._closedPromise below), and we want errors in stream to error both branches immediately. We cannot let
+        // successful synchronously-available reads get ahead of asynchronously-available errors.
+        queueMicrotask(() => {
+          reading = false;
+
+          const chunk1 = chunk;
+          let chunk2 = chunk;
+          if (canceled1 === false && canceled2 === false) {
+            chunk2 = new chunk.constructor(chunk);

Ugh, right, I'm trying to be too clever. This constructor works for typed arrays, but not for `DataView`s. 🤦‍♂️

I guess I'll have to write a helper function. 😛

> +      |r|).
+   1. If |canceled1| is false or |canceled2| is false, [=resolve=] |cancelPromise| with undefined.
+ 1. Let |pullWithDefaultReader| be the following steps:
+  1. If |reader| [=implements=] {{ReadableStreamBYOBReader}},
+   1. Assert: |reader|.[=ReadableStreamBYOBReader/[[readIntoRequests]]=] is [=list/is empty|empty=].
+   1. Perform ! [$ReadableStreamReaderGenericRelease$](|reader|).
+   1. Set |reader| to ! [$AcquireReadableStreamDefaultReader$](|stream|).
+   1. Perform |forwardReaderError|, given |reader|.
+  1. Let |readRequest| be a [=read request=] with the following [=struct/items=]:
+   : [=read request/chunk steps=], given |chunk|
+   ::
+    1. [=Queue a microtask=] to perform the following steps:
+     1. Set |reading| to false.
+     1. Let |chunk1| and |chunk2| be |chunk|.
+     1. If |canceled1| is false and |canceled2| is false, set |chunk2| to !
+        [$StructuredDeserialize$](? [$StructuredSerialize$](|chunk|), [=the current Realm=]).

Perhaps this is a bit overkill? This copies the entire backing `ArrayBuffer`, but we only really need the bytes within the `chunk`'s view. 🤔

I suppose we could make a dedicated `CloneArrayBufferView` helper, rather than using structured serialize/deserialize.

-- 
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/1114#pullrequestreview-619222774