Use new Streams algorithms #1533
Conversation
|
Drive-by lgtm. |
docs/index.bs
Outdated
| : [=read request/close steps=] | ||
| :: | ||
| 1. Set |end-of-body| to true. | ||
| : [=read request/error steps=] | ||
| 1. [=ReadableStream/error=] |newStream| with a {{TypeError}}. |
There was a problem hiding this comment.
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:
| : [=read request/close steps=] | |
| :: | |
| 1. Set |end-of-body| to true. | |
| : [=read request/error steps=] | |
| 1. [=ReadableStream/error=] |newStream| with a {{TypeError}}. | |
| : [=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?
There was a problem hiding this comment.
Thanks for catching the missing ::!
I find it clearer when steps are numbered, but we could remove it if you'd prefer; the reader can understand.
I don't think of <dd>s as "sentences", personally, but instead terms. So I don't capitalize them unless they're proper nouns.
There was a problem hiding this comment.
Fair enough. Add in the :: then it's good to go.
There was a problem hiding this comment.
Forgot to push; should be good now!
| 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|. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yeah, see speced/bikeshed#1758
There was a problem hiding this comment.
I'm ok with this, but in general I don't see the need for the vars here.
When we "Set document's foo to bar", we don't wrap foo in a <var>, so I don't see the need to do it here.
There was a problem hiding this comment.
See the discussion in whatwg/infra#320
Co-authored-by: Jake Archibald <jaffathecake@gmail.com>
Follows whatwg/streams#1073.
Do not merge until that PR is merged. But, feel free to review.
See also speced/bikeshed#1758.
Preview | Diff