Re: [whatwg/streams] Update ReadableStreamTee to allow cloning for only the second branch (#528)

tyoshino commented on this pull request.



>        // There is no way to access the cloning code right now in the reference implementation.
       // If we add one then we'll need an implementation for StructuredClone.
-
+      // if (teeState.canceled2 === false && cloneForBranch2 === true) {
+      //   value2 = StructuredClone(value2);
+      // }

Even when the branch 1 has been cancelled, we clone values. I think it's ok and is more consistent, but leaving this comment to make sure you're aware of.

> @@ -782,14 +769,11 @@ ReadableStreamTee pull function <var>F</var> is called, it performs the followin
         1. Perform ! ReadableStreamDefaultControllerClose(_branch2_).
       1. Set _teeState_.[[closedOrErrored]] to *true*.
     1. If _teeState_.[[closedOrErrored]] is *true*, return *undefined*.
-    1. If _teeState_.[[canceled1]] is *false*,
-      1. Let _value1_ be _value_.
-      1. If _shouldClone_ is *true*, set _value1_ to ? <a abstract-op>StructuredClone</a>(_value_).
-      1. Perform ? ReadableStreamDefaultControllerEnqueue(_branch1_, _value1_).
-    1. If _teeState_.[[canceled2]] is *false*,
-      1. Let _value2_ be _value_.
-      1. If _shouldClone_ is *true*, set _value2_ to ? <a abstract-op>StructuredClone</a>(_value_).
-      1. Perform ? ReadableStreamDefaultControllerEnqueue(_branch2_, _value2_).
+    1. Let _value1_ and _value2_ be _value_.
+    1. If _teeState_.[[canceled2]] is *false* and _cloneForBranch2_ is *true*, set _value2_ to ? <a
+       abstract-op>StructuredClone</a>(_value2_).
+    1. If _teeState_.[[canceled1]] is *false*, perform ? ReadableStreamDefaultControllerEnqueue(_branch1_, _value1_).
+    1. If _teeState_.[[canceled2]] is *false*, perform ? ReadableStreamDefaultControllerEnqueue(_branch1_, _value2_).

_branch2_

> @@ -28,11 +28,6 @@ Ignored Vars: e
 </pre>
 
 <pre class="anchors">
-urlPrefix: https://html.spec.whatwg.org/multipage/; spec: HTML
-    text: StructuredClone; urlPrefix: infrastructure.html; type: abstract-op
-    text: Transfer; url: infrastructure.html#transfer-abstract-op; type: abstract-op
-    text: WebSocket; url: comms.html#websocket; type: interface
-    text: transferable objects; url: infrastructure.html; type: dfn

what made these unnecessary?

-- 
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/528#pullrequestreview-3636190