Re: [whatwg/dom] add signal to addEventListener (#919)

@domenic commented on this pull request.



> @@ -962,6 +966,7 @@ dictionary EventListenerOptions {
 dictionary AddEventListenerOptions : EventListenerOptions {
   boolean passive = false;
   boolean once = false;
+  AbortSignal? signal;

I think the IDL should not be nullable. Since this is not marked `required`, you can already omit it (or pass undefined). Additionally allowing passing `null` is not great, and is something we've avoided with other APIs.

>  
  <li><p>If <var>options</var> is a dictionary, then set <var>passive</var> to <var>options</var>'s
- <code>{{AddEventListenerOptions/passive}}</code> and <var>once</var> to <var>options</var>'s
- <code>{{AddEventListenerOptions/once}}</code>.
+ <code>{{AddEventListenerOptions/passive}}</code>, <var>once</var> to <var>options</var>'s
+ <code>{{AddEventListenerOptions/once}}</code> and <var>signal</var> to
+ {{AddEventListenerOptions/signal}}.

If we follow my above advice and remove the nullability, then we'll need to break this out into a separate step:

```html
If |options| is a dictionary and |options|["{{AddEventListenerOptions/signal}}"] [=map/exists=], then set |signal| to |options|["{{AddEventListenerOptions/signal}}"].
```

> @@ -1113,6 +1127,12 @@ participate in a tree structure.</p>
  <a for="event listener">callback</a>, and <a for="event listener">capture</a> is
  <var>listener</var>'s <a for="event listener">capture</a>, then <a for=list>append</a>
  <var>listener</var> to <var>eventTarget</var>'s <a>event listener list</a>.
+
+  <li><p>If <a for="event listener">signal</a> is not null then <a for=AbortSignal lt=add>add the following</a>
+  abort steps to it:
+    <ol>
+    <li><a>remove an event listener</a> with <var>eventTarget</var> and <var>listener</var>.
+    </ol>

Nit: the indentation here is not quite the same as other nested steps.

-- 
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/dom/pull/919#pullrequestreview-527408761