Re: [w3c/ServiceWorker] Use new Streams algorithms (#1533)

@jakearchibald requested changes on this pull request.



> +                            : [=read request/close steps=]
+                            ::
+                                1. Set |end-of-body| to true.
+                            : [=read request/error steps=]
+                                1. [=ReadableStream/error=] |newStream| with a {{TypeError}}.

The markup seems inconsistent here. With the error steps, the steps aren't in a definition item. Given that they're both a single step, do they need to be in an ordered list at all? Should it be:

```suggestion
                            : [=read request/close steps=]
                            :: Set |end-of-body| to true.
                            : [=read request/error steps=]
                            :: [=ReadableStream/error=] |newStream| with a {{TypeError}}.
```

Also, is it typical to use lower case for "close steps" and "error" even though they're at the beginning of a sentence?

>                      1. Let |highWaterMark| be a non-negative, non-NaN number, chosen by the user agent.
                     1. Let |sizeAlgorithm| be an algorithm that accepts a [=chunk=] object and returns a non-negative, non-NaN, non-infinite number, chosen by the user agent.
-                    1. Let |pull| be an action that runs these subsubsteps:
-                        1. Let |promise| be the result of [=read a chunk|reading a chunk=] from |response|'s [=response/body=]'s [=stream=] with |reader|.
-                        1. When |promise| is fulfilled with an object whose `done` property is false and whose `value` property is a `Uint8Array` object, append the bytes represented by the `value` property to |bytes| and perform ! [=DetachArrayBuffer=] with the `ArrayBuffer` object wrapped by the `value` property.
-                        1. When |promise| is fulfilled with an object whose `done` property is true, set |end-of-body| to true.
-                        1. When |promise| is fulfilled with a value that matches with neither of the above patterns, or |promise| is rejected, [=ReadableStream/error=] |newStream| with a `TypeError`.
-                    1. Let |cancel| be an action that [=ReadableStream/cancels=] |response|'s [=response/body=]'s [=stream=] with |reader|.
-                    1. Let |newStream| be the result of [=ReadableStream/construct a ReadableStream object=] with |highWaterMark|, |sizeAlgorithm|, |pull|, and |cancel| in |targetRealm|.
+                    1. Let |newStream| be the result of [=ReadableStream/creating=] a {{ReadableStream}} with <a for=ReadableStream/create><var ignore>pullAlgorithm</var></a> set to |pullAlgorithm|, <a for=ReadableStream/create><var ignore>cancelAlgorithm</var></a> set to |cancelAlgorithm|, <a for=ReadableStream/create><var ignore>highWaterMark</var></a> set to |highWaterMark|, and <a for=ReadableStream/create><var ignore>sizeAlgorithm</var></a> set to |sizeAlgorithm|, in |targetRealm|.

Huh, was this the agreed markup for this? It seems weird to create a var that you have to ignore. And of course it doesn't work with bikeshed's shorthand.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/ServiceWorker/pull/1533#pullrequestreview-486594963