Skip to content

Commit

Permalink
events: support signal in EventTarget
Browse files Browse the repository at this point in the history
PR-URL: #36258
Fixes: #36073
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
benjamingr authored and nodejs-github-bot committed Dec 2, 2020
1 parent 4a741b8 commit ef8d0e9
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 0 deletions.
22 changes: 22 additions & 0 deletions lib/internal/event_target.js
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,12 @@ class Listener {
previous.next = this;
this.previous = previous;
this.listener = listener;
// TODO(benjamingr) these 4 can be 'flags' to save 3 slots
this.once = once;
this.capture = capture;
this.passive = passive;
this.isNodeStyleListener = isNodeStyleListener;
this.removed = false;

this.callback =
typeof listener === 'function' ?
Expand All @@ -220,6 +222,7 @@ class Listener {
this.previous.next = this.next;
if (this.next !== undefined)
this.next.previous = this.previous;
this.removed = true;
}
}

Expand Down Expand Up @@ -269,6 +272,7 @@ class EventTarget {
once,
capture,
passive,
signal,
isNodeStyleListener
} = validateEventListenerOptions(options);

Expand All @@ -286,6 +290,17 @@ class EventTarget {
}
type = String(type);

if (signal) {
if (signal.aborted) {
return false;
}
// TODO(benjamingr) make this weak somehow? ideally the signal would
// not prevent the event target from GC.
signal.addEventListener('abort', () => {
this.removeEventListener(type, listener, options);
}, { once: true });
}

let root = this[kEvents].get(type);

if (root === undefined) {
Expand Down Expand Up @@ -382,6 +397,12 @@ class EventTarget {
// Cache the next item in case this iteration removes the current one
next = handler.next;

if (handler.removed) {
// Deal with the case an event is removed while event handlers are
// Being processed (removeEventListener called from a listener)
handler = next;
continue;
}
if (handler.once) {
handler.remove();
root.size--;
Expand Down Expand Up @@ -550,6 +571,7 @@ function validateEventListenerOptions(options) {
once: Boolean(options.once),
capture: Boolean(options.capture),
passive: Boolean(options.passive),
signal: options.signal,
isNodeStyleListener: Boolean(options[kIsNodeStyleListener])
};
}
Expand Down
155 changes: 155 additions & 0 deletions test/parallel/test-eventtarget-whatwg-signal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
'use strict';

require('../common');

const {
strictEqual,
} = require('assert');

// Manually ported from: wpt@dom/events/AddEventListenerOptions-signal.any.js

{
// Passing an AbortSignal to addEventListener does not prevent
// removeEventListener
let count = 0;
function handler() {
count++;
}
const et = new EventTarget();
const controller = new AbortController();
et.addEventListener('test', handler, { signal: controller.signal });
et.dispatchEvent(new Event('test'));
strictEqual(count, 1, 'Adding a signal still adds a listener');
et.dispatchEvent(new Event('test'));
strictEqual(count, 2, 'The listener was not added with the once flag');
controller.abort();
et.dispatchEvent(new Event('test'));
strictEqual(count, 2, 'Aborting on the controller removes the listener');
et.addEventListener('test', handler, { signal: controller.signal });
et.dispatchEvent(new Event('test'));
strictEqual(count, 2, 'Passing an aborted signal never adds the handler');
}

{
// Passing an AbortSignal to addEventListener works with the once flag
let count = 0;
function handler() {
count++;
}
const et = new EventTarget();
const controller = new AbortController();
et.addEventListener('test', handler, { signal: controller.signal });
et.removeEventListener('test', handler);
et.dispatchEvent(new Event('test'));
strictEqual(count, 0, 'The listener was still removed');
}

{
// Removing a once listener works with a passed signal
let count = 0;
function handler() {
count++;
}
const et = new EventTarget();
const controller = new AbortController();
const options = { signal: controller.signal, once: true };
et.addEventListener('test', handler, options);
controller.abort();
et.dispatchEvent(new Event('test'));
strictEqual(count, 0, 'The listener was still removed');
}

{
let count = 0;
function handler() {
count++;
}
const et = new EventTarget();
const controller = new AbortController();
const options = { signal: controller.signal, once: true };
et.addEventListener('test', handler, options);
et.removeEventListener('test', handler);
et.dispatchEvent(new Event('test'));
strictEqual(count, 0, 'The listener was still removed');
}

{
// Passing an AbortSignal to multiple listeners
let count = 0;
function handler() {
count++;
}
const et = new EventTarget();
const controller = new AbortController();
const options = { signal: controller.signal, once: true };
et.addEventListener('first', handler, options);
et.addEventListener('second', handler, options);
controller.abort();
et.dispatchEvent(new Event('first'));
et.dispatchEvent(new Event('second'));
strictEqual(count, 0, 'The listener was still removed');
}

{
// Passing an AbortSignal to addEventListener works with the capture flag
let count = 0;
function handler() {
count++;
}
const et = new EventTarget();
const controller = new AbortController();
const options = { signal: controller.signal, capture: true };
et.addEventListener('test', handler, options);
controller.abort();
et.dispatchEvent(new Event('test'));
strictEqual(count, 0, 'The listener was still removed');
}

{
// Aborting from a listener does not call future listeners
let count = 0;
function handler() {
count++;
}
const et = new EventTarget();
const controller = new AbortController();
const options = { signal: controller.signal };
et.addEventListener('test', () => {
controller.abort();
}, options);
et.addEventListener('test', handler, options);
et.dispatchEvent(new Event('test'));
strictEqual(count, 0, 'The listener was still removed');
}

{
// Adding then aborting a listener in another listener does not call it
let count = 0;
function handler() {
count++;
}
const et = new EventTarget();
const controller = new AbortController();
et.addEventListener('test', () => {
et.addEventListener('test', handler, { signal: controller.signal });
controller.abort();
}, { signal: controller.signal });
et.dispatchEvent(new Event('test'));
strictEqual(count, 0, 'The listener was still removed');
}

{
// Aborting from a nested listener should remove it
const et = new EventTarget();
const ac = new AbortController();
let count = 0;
et.addEventListener('foo', () => {
et.addEventListener('foo', () => {
count++;
if (count > 5) ac.abort();
et.dispatchEvent(new Event('foo'));
}, { signal: ac.signal });
et.dispatchEvent(new Event('foo'));
}, { once: true });
et.dispatchEvent(new Event('foo'));
}
9 changes: 9 additions & 0 deletions test/parallel/test-eventtarget.js
Original file line number Diff line number Diff line change
Expand Up @@ -532,3 +532,12 @@ let asyncTest = Promise.resolve();
target.dispatchEvent(new Event('foo'));
deepStrictEqual(output, [1, 2, 3, 4]);
}
{
const et = new EventTarget();
const listener = common.mustNotCall();
et.addEventListener('foo', common.mustCall((e) => {
et.removeEventListener('foo', listener);
}));
et.addEventListener('foo', listener);
et.dispatchEvent(new Event('foo'));
}

0 comments on commit ef8d0e9

Please sign in to comment.