Re: [whatwg/streams] A variety of clarity and documentation improvements (#875)

surma requested changes on this pull request.

Very happy with this! One nit and some suggestions for even moar links ;)

> +    <p>A function that is called after the <a>producer</a> signals, via {{WritableStream/abort()|stream.abort()}} or
+    {{WritableStreamDefaultWriter/abort()|writer.abort()}}, that they wish to abruptly close the stream and put it in an
+    errored state, discarding any queued-up writes. It takes as its argument the same value as was passed to those
+    methods by the producer.</p>
+
+    <p>This function can clean up any held resources, much like {{underlying sink/close()}}, but perhaps with some
+    custom handling.</p>
+
+    <p>If the shutdown process is asynchronous, the function can return a promise to signal success or failure; the
+    result will be communicated via the return value of the called <code>abort()</code> method. Throwing an exception is
+    treated the same as returning a rejected promise. Regardless, the stream will be errored with a new {{TypeError}}
+    indicating that it was aborted.</p>
+  </dd>
+</dl>
+
+The <code>controller</code> argument passed to {{underlying sink/start()}} and {{underlying sink/write()}} is an

I just stumbled over this. It’s pretty easy to miss. Could we move it to the top or maybe even duplicate it to the individual methods that take `controller`? I know this is a spec and not developer documentation, so you can make the call here, just giving a developer’s perspective on this.

> @@ -502,6 +469,85 @@ ReadableStream(<var>underlyingSource</var> = {}, { <var>size</var>, <var>highWat
   1. Otherwise, throw a *RangeError* exception.
 </emu-alg>
 
+<h4 id="underlying-source-api">Underlying Source API</h4>
+
+The {{ReadableStream()}} constructor accepts as its first argument a JavaScript object representing the <a>underlying
+source</a>. Such objects may contain any of the following properties:
+
+<dl>
+  <dt><dfn method for="underlying source">start(<var>controller</var>)</dfn></dt>

`controller` is not defined in this section. At the very least there should have a paragraph similar to the one in “Underlying Sink API”. 

> @@ -4087,6 +4163,66 @@ readableStrategy)">new TransformStream(<var>transformer</var> = {}, <var>writabl
   1. <a>Resolve</a> _startPromise_ with _startResult_.
 </emu-alg>
 
+<h4 id="transformer-api">Transformer API</h4>

This should also be directly linked to by “2.3. Transform Streams” imo

-- 
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/875#pullrequestreview-93110496

Received on Wednesday, 31 January 2018 22:47:43 UTC