Re: [whatwg/streams] Add support for ReadableStream "owning" type (PR #1271)

@ricea commented on this pull request.

This is looking good.

I thought I saw you had updated the non-normative `domintro` description of `enqueue` to include the new parameter, but I can't see it now. If it's not there, could you add it? I'd put this comment on line 1557 but github won't let me.

We will need an example of using an owning stream (but no need to add one now).

> @@ -40,6 +42,18 @@ exports.PeekQueueValue = container => {
 exports.ResetQueue = container => {
   assert('_queue' in container && '_queueTotalSize' in container);
 
+  if (container._isOwning) {
+    while (container._queue.length > 0) {
+      const value = exports.DequeueValue(container);
+      if (typeof value.close === 'function') {

I'm not sure about calling the "close" method. It will often do the right thing, but it's different enough from the specced behaviour that it might cause confusion. I would probably use some private symbol instead of a literal "close", something like this:
```js
if (typeof value[closingSteps] === 'function') {
  try {
    value[closingSteps]();
  } catch {}
}
```
although that would make this functionality challenging to test.

Opinions?

> @@ -2950,13 +2968,19 @@ The following abstract operations support the implementation of the
 <div algorithm>
  <dfn abstract-op lt="ReadableStreamDefaultControllerEnqueue"
  id="readable-stream-default-controller-enqueue">ReadableStreamDefaultControllerEnqueue(|controller|,
- |chunk|)</dfn> performs the following steps:
+ |chunk|, |transferList|)</dfn> performs the following steps:
 
  1. If ! [$ReadableStreamDefaultControllerCanCloseOrEnqueue$](|controller|) is false, return.

Generally when the public API has a precondition, we put an assert in the private API to enforce the precondition (to provide a hint to implementers and other specs). Something like
> Assert: |transferList| is empty or |controller|.[[isOwning]] is true.

>   following steps:
 
  1. Assert: |container| has \[[queue]] and \[[queueTotalSize]] internal slots.
  1. If ! [$IsNonNegativeNumber$](|size|) is false, throw a {{RangeError}} exception.
  1. If |size| is +∞, throw a {{RangeError}} exception.
- 1. [=list/Append=] a new [=value-with-size=] with [=value-with-size/value=] |value| and
+ 1. Let |enqueuedValue| be |value|.
+ 1. If |container| has a \[[isOwning]] internal slot whose value is true, perform the following steps:

```suggestion
 1. If |container| has an \[[isOwning]] internal slot whose value is true, perform the following steps:
```

> @@ -6388,6 +6419,10 @@ for="value-with-size">value</dfn> and <dfn for="value-with-size">size</dfn>.
  performs the following steps:
 
  1. Assert: |container| has \[[queue]] and \[[queueTotalSize]] internal slots.
+ 1. If |container| has a \[[isOwning]] internal slot whose value is true, perform the following steps until |container|.\[[queue]]

```suggestion
 1. If |container| has an \[[isOwning]] internal slot whose value is true, perform the following steps until |container|.\[[queue]]
```

> @@ -6596,6 +6631,14 @@ The following abstract operations are a grab-bag of utilities.
  1. Return ? [$StructuredDeserialize$](|serialized|, [=the current Realm=]).
 </div>
 
+<div algorithm>
+ <dfn abstract-op lt="StructuredTransferOrClone">StructuredTransferOrClone(|value|, |transferList|)</dfn>
+    performs the following steps:
+ 1. Let |serialized| be ! [$StructuredSerializeWithTransfer$](|value|, |transferList|).

Both StructuredSerializeWithTransfer and StructuredDeserializeWithTransfer can throw, so they should have **?** in front of them, not **!**.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/1271#pullrequestreview-1392394695
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/streams/pull/1271/review/1392394695@github.com>