From 866e7af63590d9c221b89263adc7ef082df5b9ff Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Sat, 31 Oct 2020 11:39:50 +0200 Subject: [PATCH 1/3] events: support abortsignal in ee.on --- doc/api/events.md | 28 ++++++-- lib/events.js | 31 +++++++-- .../test-event-emitter-remove-listeners.js | 68 +++++++++++++++++++ 3 files changed, 116 insertions(+), 11 deletions(-) diff --git a/doc/api/events.md b/doc/api/events.md index 17420a249be731..9e86e0431b13cb 100644 --- a/doc/api/events.md +++ b/doc/api/events.md @@ -383,15 +383,17 @@ Installing a listener using this symbol does not change the behavior once an `'error'` event is emitted, therefore the process will still crash if no regular `'error'` listener is installed. -### `emitter.addListener(eventName, listener)` +### `emitter.addListener(eventName, listener [,options])` * `eventName` {string|symbol} * `listener` {Function} +* `options` {Object} + * `signal` {AbortSignal} Can be used to unsubscribe from the event -Alias for `emitter.on(eventName, listener)`. +Alias for `emitter.on(eventName, listener, options)`. ### `emitter.emit(eventName[, ...args])` * `eventName` {string|symbol} The name of the event. * `listener` {Function} The callback function +* `options` {Object} + * `signal` {AbortSignal} Can be used to unsubscrie from the event * Returns: {EventEmitter} Adds the `listener` function to the end of the listeners array for the @@ -556,6 +560,20 @@ myEE.emit('foo'); // a ``` +An AbortSignal can be passed in to facilitate removal of the listener without +keeping a reference to the listener function: + +```js +const myEE = new EventEmitter(); +const ac = new AbortController(); +myEE.on('foo', () => console.log('a'), { signal: ac.signal }); +myEE.emit('foo'); +// Prints: +// a +ac.abort(); +myEE.emit('foo'); +// Prints nothing, the listener was removed +``` ### `emitter.once(eventName, listener)` * `eventName` {string|symbol} The name of the event. * `listener` {Function} The callback function +* `options` {Object} + * `signal` {AbortSignal} Can be used to unsubscrie from the event * Returns: {EventEmitter} Adds the `listener` function to the *beginning* of the listeners array for the diff --git a/lib/events.js b/lib/events.js index f82776072b1598..bac697e5aab09d 100644 --- a/lib/events.js +++ b/lib/events.js @@ -352,13 +352,30 @@ EventEmitter.prototype.emit = function emit(type, ...args) { return true; }; -function _addListener(target, type, listener, prepend) { +function _addListener(target, type, listener, prepend, options) { let m; let events; let existing; checkListener(listener); + if (options !== undefined) { + const signal = options ? options.signal : undefined; + validateAbortSignal(signal, 'options.signal'); + if (signal.aborted) { + return target; + } + // The same listener can be added/removed multiple times + // the signal needs to remove the correct instance - so we wrap it + const originalListener = listener; + listener = function listener(...args) { + return originalListener.apply(this, args); + }; + listener.listener = originalListener; + signal.addEventListener('abort', () => { + target.removeListener(type, listener); + }, { once: true }); + } events = target._events; if (events === undefined) { events = target._events = ObjectCreate(null); @@ -414,15 +431,16 @@ function _addListener(target, type, listener, prepend) { return target; } -EventEmitter.prototype.addListener = function addListener(type, listener) { - return _addListener(this, type, listener, false); -}; +EventEmitter.prototype.addListener = + function addListener(type, listener, options) { + return _addListener(this, type, listener, false, options); + }; EventEmitter.prototype.on = EventEmitter.prototype.addListener; EventEmitter.prototype.prependListener = - function prependListener(type, listener) { - return _addListener(this, type, listener, true); + function prependListener(type, listener, options) { + return _addListener(this, type, listener, true, options); }; function onceWrapper() { @@ -506,7 +524,6 @@ EventEmitter.prototype.removeListener = if (events.removeListener !== undefined) this.emit('removeListener', type, listener); } - return this; }; diff --git a/test/parallel/test-event-emitter-remove-listeners.js b/test/parallel/test-event-emitter-remove-listeners.js index f37d26eb258c23..181eef47c15f00 100644 --- a/test/parallel/test-event-emitter-remove-listeners.js +++ b/test/parallel/test-event-emitter-remove-listeners.js @@ -168,3 +168,71 @@ function listener2() {} ee.removeListener('foo', listener1); assert.strictEqual(ee._events.foo, listener2); } +{ + // AbortSignals + const ac = new AbortController(); + const ee = new EventEmitter(); + ee.on('foo', common.mustNotCall(), { signal: ac.signal }); + ee.prependListener('foo', common.mustNotCall(), { signal: ac.signal }); + ac.abort(); + ee.emit('foo'); +} +{ + // AbortSignals - aborting before adding the listener + const ac = new AbortController(); + const ee = new EventEmitter(); + ac.abort(); + ee.on('foo', common.mustNotCall(), { signal: ac.signal }); + ee.prependListener('foo', common.mustNotCall(), { signal: ac.signal }); + ee.emit('foo'); +} +{ + // AbortSignals - removing the correct listener + const ac = new AbortController(); + const ee = new EventEmitter(); + const mustCall = common.mustCall(); + const mustNotCall = common.mustNotCall(); + ee.on('foo', mustNotCall, { signal: ac.signal }); + ee.on('foo', mustCall); + ac.abort(); + ee.emit('foo'); +} +{ + // AbortSignals - removing the correct same listener + const ac = new AbortController(); + const ee = new EventEmitter(); + const arr = []; + const fn1 = (arg) => arr.push(arg); + const pushA = fn1.bind(null, 'A'); + const pushB = fn1.bind(null, 'B'); + ee.on('foo', pushA, { signal: ac.signal }); + ee.on('foo', pushB); + ee.on('foo', pushA); + ac.abort(); + ee.emit('foo'); + assert.deepStrictEqual(arr, ['B', 'A']); +} +{ + // AbortSignals - removing the correct same listener + const ac = new AbortController(); + const ee = new EventEmitter(); + const arr = []; + const fn1 = (arg) => arr.push(arg); + const pushA = fn1.bind(null, 'A'); + const pushB = fn1.bind(null, 'B'); + ee.on('foo', pushA); + ee.on('foo', pushB); + ee.on('foo', pushA, { signal: ac.signal }); + ac.abort(); + ee.emit('foo'); + assert.deepStrictEqual(arr, ['A', 'B']); +} +{ + // AbortSignals - removing with removeListener + const ac = new AbortController(); + const ee = new EventEmitter(); + const mustNotCall = common.mustNotCall(); + ee.on('foo', mustNotCall, { signal: ac.signal }); + ee.removeListener('foo', mustNotCall); + ee.emit('foo'); +} \ No newline at end of file From c9e2a829a2130f675819b4d1ff1bcf6ce54bf767 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Sat, 31 Oct 2020 11:46:48 +0200 Subject: [PATCH 2/3] fixup! add test --- doc/api/events.md | 2 +- .../test-event-emitter-remove-listeners.js | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/doc/api/events.md b/doc/api/events.md index 9e86e0431b13cb..f38c25610ee1b8 100644 --- a/doc/api/events.md +++ b/doc/api/events.md @@ -560,7 +560,7 @@ myEE.emit('foo'); // a ``` -An AbortSignal can be passed in to facilitate removal of the listener without +An AbortSignal can be passed in to facilitate removal of the listener without keeping a reference to the listener function: ```js diff --git a/test/parallel/test-event-emitter-remove-listeners.js b/test/parallel/test-event-emitter-remove-listeners.js index 181eef47c15f00..b2bd5cc38c2870 100644 --- a/test/parallel/test-event-emitter-remove-listeners.js +++ b/test/parallel/test-event-emitter-remove-listeners.js @@ -23,6 +23,7 @@ const common = require('../common'); const assert = require('assert'); const EventEmitter = require('events'); +const { strictEqual } = require('assert'); function listener1() {} @@ -235,4 +236,18 @@ function listener2() {} ee.on('foo', mustNotCall, { signal: ac.signal }); ee.removeListener('foo', mustNotCall); ee.emit('foo'); -} \ No newline at end of file +} +{ + // AbortSignals - checking rawListeners + const ac = new AbortController(); + const ee = new EventEmitter(); + const fn = common.mustCall(); + ee.on('foo', fn, { signal: ac.signal }); + const listeners = ee.listeners('foo'); + strictEqual(listeners.length, 1); + strictEqual(listeners[0], fn); + ee.emit('foo'); + const rawListeners = ee.rawListeners('foo'); + strictEqual(rawListeners.length, 1); + strictEqual(rawListeners[0].listener, fn); +} From 96227468562a22fe7c32fd0639d621dd34a67748 Mon Sep 17 00:00:00 2001 From: Benjamin Gruenbaum Date: Sat, 31 Oct 2020 19:14:20 +0200 Subject: [PATCH 3/3] fixup! add test + guard for undefined --- lib/events.js | 2 +- test/parallel/test-event-emitter-remove-listeners.js | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/events.js b/lib/events.js index bac697e5aab09d..c3828539fb31c5 100644 --- a/lib/events.js +++ b/lib/events.js @@ -362,7 +362,7 @@ function _addListener(target, type, listener, prepend, options) { if (options !== undefined) { const signal = options ? options.signal : undefined; validateAbortSignal(signal, 'options.signal'); - if (signal.aborted) { + if (signal && signal.aborted) { return target; } // The same listener can be added/removed multiple times diff --git a/test/parallel/test-event-emitter-remove-listeners.js b/test/parallel/test-event-emitter-remove-listeners.js index b2bd5cc38c2870..5b75842ef91f33 100644 --- a/test/parallel/test-event-emitter-remove-listeners.js +++ b/test/parallel/test-event-emitter-remove-listeners.js @@ -251,3 +251,14 @@ function listener2() {} strictEqual(rawListeners.length, 1); strictEqual(rawListeners[0].listener, fn); } +{ + // AbortSignals - already aborted + const ac = new AbortController(); + const ee = new EventEmitter(); + const fn = common.mustNotCall(); + ac.abort(); + ee.on('foo', fn, { signal: ac.signal }); + const listeners = ee.listeners('foo'); + strictEqual(listeners.length, 0); + ee.emit('foo'); +}