Re: [whatwg/dom] Add AbortSignal.any() (PR #1152)

@annevk commented on this pull request.

I've done a quick editorial take. This would probably benefit from review by @smaug----, @jakearchibald, and @domenic. 

> @@ -1789,6 +1792,7 @@ to <a for=AbortSignal>signal abort</a> on <a>this</a>'s <a for=AbortController>s
 interface AbortSignal : EventTarget {
   [NewObject] static AbortSignal abort(optional any reason);
   [Exposed=(Window,Worker), NewObject] static AbortSignal timeout([EnforceRange] unsigned long long milliseconds);
+  [NewObject] static AbortSignal _any(sequence&lt;AbortSignal> signals);

I'm somewhat surprised that `Promise.any` takes an iterable and not rest parameters.

> @@ -1779,8 +1779,11 @@ constructor steps are:
 <a>this</a>'s <a for=AbortController>signal</a>.
 
 <p>The <dfn method for=AbortController><code>abort(<var>reason</var>)</code></dfn> method steps are
-to <a for=AbortSignal>signal abort</a> on <a>this</a>'s <a for=AbortController>signal</a> with
-<var>reason</var> if it is given.
+to <a for=AbortController>request abort</a> on <a>this</a> with <var>reason</var> if it is given.

Since it's not really a request, maybe just call this abort or also signal abort? (The for attribute helps with disambiguation and we can just not export the other one.)

> @@ -1802,6 +1806,11 @@ interface AbortSignal : EventTarget {
  <dd>Returns an {{AbortSignal}} instance whose <a for=AbortSignal>abort reason</a> is set to
  <var>reason</var> if not undefined; otherwise to an "{{AbortError!!exception}}" {{DOMException}}.
 
+ <dt><code>AbortSignal . <a method for=AbortSignal lt=any(signals)>any</a>(<var>signals</var>)</code>
+ <dd>Returns an {{AbortSignal}} instance which will be aborted if any of <var>signals</var> is

once any of*?

>  them. For instance, if the operation has already completed.
 
-<p>To <dfn export for=AbortSignal>signal abort</dfn>, given an {{AbortSignal}} object
+<p>To <dfn noexport for=AbortSignal>signal abort</dfn>, given an {{AbortSignal}} object

Removing the export attribute should suffice.

>  </ol>
 
-<p>A <var>followingSignal</var> (an {{AbortSignal}}) is made to
-<dfn export for=AbortSignal>follow</dfn> a <var>parentSignal</var> (an {{AbortSignal}}) by running
-these steps:
+<p>To <dfn export for=AbortSignal>create a composite abort signal</dfn> from a list of

Let's drop the `for` attribute here. It's not really a member of an AbortSignal and it's unique enough.

> +{{AbortSignal}} objects <var>signals</var> or a single {{AbortSignal}} <var>signal</var>, an
+optional <var>signalInterface</var>, which must be either {{AbortSignal}} or an interface that
+inherits from it, and an optional <var>realm</var>, run these steps:

This is confusing. Let's always require a list. Folks can use the list syntax from Infra.

And do we want an interface or some kind of creation algorithm? (Also, if you want it to default to AbortSignal you should specify that here using the language from Infra, but please only do that if there are many callers. Otherwise it's not really worth it.)

>  
- <li>
-  <p>Otherwise, <a for=AbortSignal lt=add>add the following abort steps</a> to
-  <var>parentSignal</var>:
+ <li><p>If <var>signal</var> was given, then set <var>signals</var> to a new <a for="/">list</a> and
+ <a for=list>append</a> <var>signal</var> to it.
+
+ <li><p><a for=list>For each</a> <var>signal</var> of <var>signals</var>: if <var>signal</var> is
+ [=AbortSignal/aborted=], then set <var>resultSignal</var>'s [=AbortSignal/abort reason=] to
+ <var>signal</var>'s [=AbortSignal/abort reason=] and return <var>resultSignal</var>.
+ </li>

No need for `</li>` tags.

>  
+ <li><p><a for=list>For each</a> <var>signal</var> of <var>signals</var>:

`<p>` needs to go on a newline when `<li>` contains multiple children. Applies multiple times.

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

Message ID: <whatwg/dom/pull/1152/review/1286782345@github.com>