Re: [whatwg/streams] Editorial: add cross-links to internal slots (#1050)

@MattiasBuelens commented on this pull request.

Well, I tried something. It's very rough, so I'd very much like to hear your suggestions! 😄

Honestly, I think we'd be better off with an actual `interface mixin ReadableStreamGenericReader`. That way, we could move the shared internal slots *and* the shared `closed` getter and `cancel(reason)` method to that mixin. But I suppose such a change is no longer merely editorial, and would need a separate PR?

(I know the lines are too long, I'll fix those later.)

>   1. If |reader| is undefined, return false.
  1. If |reader| [=implements=] {{ReadableStreamDefaultReader}}, return true.
  1. Return false.
 </div>
 
 <h4 id="rs-reader-abstract-ops">Readers</h4>
 
+Instances of {{ReadableStreamDefaultReader}} and {{ReadableStreamBYOBReader}} are created with the
+internal slots described in the following table:

Is it weird to talk about "instances are created with the internal slots described in the following table" here, when actually they also have additional slots (i.e. `[[readRequests]]` and `[[readIntoRequests]]`) defined by the specific class as well?

> @@ -2401,14 +2405,31 @@ the {{ReadableStream}}'s public API.
  id="readable-stream-has-default-reader">ReadableStreamHasDefaultReader(|stream|)</dfn> performs the
  following steps:
 
- 1. Let |reader| be |stream|.\[[reader]].
+ 1. Let |reader| be |stream|.[=ReadableStream/[[reader]]=].
  1. If |reader| is undefined, return false.
  1. If |reader| [=implements=] {{ReadableStreamDefaultReader}}, return true.
  1. Return false.
 </div>
 
 <h4 id="rs-reader-abstract-ops">Readers</h4>

I added the shared internal slots to the "abstract operations" section for readers. Not sure if this is the best place for it. 🤷 

>   1. If |reader| is undefined, return false.
  1. If |reader| [=implements=] {{ReadableStreamDefaultReader}}, return true.
  1. Return false.
 </div>
 
 <h4 id="rs-reader-abstract-ops">Readers</h4>
 
+Instances of {{ReadableStreamDefaultReader}} and {{ReadableStreamBYOBReader}} are created with the
+internal slots described in the following table:
+
+<table dfn-for="ReadableStreamGenericReader">
+ <thead>
+  <tr>
+   <th>Internal Slot
+   <th>Description (<em>non-normative</em>)
+ <tbody>
+  <tr>
+   <td><dfn>\[[closedPromise]]</dfn>
+   <td class="non-normative">A promise returned by the reader's <code>closed</code> getter

I can't link to the correct `closed` getter here, since there are once again two versions... 😭

I'm starting to think that adding an IDL interface that defines `.closed` and `.cancel(reason)` would make more sense. 🤔 

>     <td class="non-normative">A promise returned by the reader's
-   {{ReadableStreamDefaultReader/closed}} getter
+   {{ReadableStreamDefaultReader/closed}} getter.
+   This fulfills the [=ReadableStreamGenericReader/[[closedPromise]]=] contract.

This is probably not a legal construct. 😛

> @@ -1131,14 +1133,14 @@ to filling the [=readable stream=]'s [=internal queue=] or changing its state. I
  The <dfn id="default-reader-closed" attribute for="ReadableStreamDefaultReader">closed</dfn>
  getter steps are:
 
- 1. Return [=this=].\[[closedPromise]].
+ 1. Return [=this=].[=ReadableStreamDefaultReader/[[closedPromise]]=].

I refer to the specific class's `[[closedPromise]]` where possible, and only refer to the generic one when both types of readers could be passed in.

Not sure if this is worth the trouble? Maybe we should remove the `<dfn>\[[closedPromise]]</dfn>` on the specific readers, and only refer to the generic one? But then what should we put in the internal slots of the specific reader classes?

-- 
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/1050#pullrequestreview-462840455