From 56ae0ffc7c2c37be77dc5d0df109a2e3bf84c7ac Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 30 May 2020 16:02:22 -0700 Subject: [PATCH] events: remove NodeEventTarget The extending of EventTarget with EventEmitter emulation is contentious and not something that is strictly necessary for minimal support. Signed-off-by: James M Snell --- doc/api/events.md | 135 ------------------------------ lib/internal/event_target.js | 114 +++---------------------- test/parallel/test-eventtarget.js | 135 +----------------------------- 3 files changed, 13 insertions(+), 371 deletions(-) diff --git a/doc/api/events.md b/doc/api/events.md index 1f25bb440122a0..84c7db5e51adea 100644 --- a/doc/api/events.md +++ b/doc/api/events.md @@ -970,27 +970,6 @@ There are two key differences between the Node.js `EventTarget` and the will be automatically captured and handled the same way as a listener that throws synchronously (see [`EventTarget` Error Handling][] for details). -### `NodeEventTarget` vs. `EventEmitter` - -The `NodeEventTarget` object implements a modified subset of the -`EventEmitter` API that allows it to closely *emulate* an `EventEmitter` in -certain situations. It is important to understand, however, that an -`NodeEventTarget` is *not* an instance of `EventEmitter` and cannot be used in -place of an `EventEmitter` in most cases. - -1. Unlike `EventEmitter`, any given `listener` can be registered at most once - per event `type`. Attempts to register a `listener` multiple times will be - ignored. -2. The `NodeEventTarget` does not emulate the full `EventEmitter` API. - Specifically the `prependListener()`, `prependOnceListener()`, - `rawListeners()`, `setMaxListeners()`, `getMaxListeners()`, and - `errorMonitor` APIs are not emulated. The `'newListener'` and - `'removeListener'` events will also not be emitted. -3. The `NodeEventTarget` does not implement any special default behavior - for events with type `'error'`. -3. The `NodeEventTarget` supports `EventListener` objects as well as - functions as handlers for all event types. - ### Event Listener Event listeners registered for an event `type` may either be JavaScript @@ -1277,120 +1256,6 @@ added: REPLACEME Removes the `listener` from the list of handlers for event `type`. -### Class: `NodeEventTarget extends EventTarget` - - -The `NodeEventTarget` is a Node.js-specific extension to `EventTarget` -that emulates a subset of the `EventEmitter` API. - -#### `nodeEventTarget.addListener(type, listener[, options])` - - -* `type` {string} -* `listener` {Function|EventListener} -* `options` {Object} - * `once` {boolean} - -* Returns: {EventTarget} this - -Node.js-specific extension to the `EventTarget` class that emulates the -equivalent `EventEmitter` API. The only difference between `addListener()` and -`addEventListener()` is that `addListener()` will return a reference to the -`EventTarget`. - -#### `nodeEventTarget.eventNames()` - - -* Returns: {string[]} - -Node.js-specific extension to the `EventTarget` class that returns an array -of event `type` names for which event listeners are currently registered. - -#### `nodeEventTarget.listenerCount(type)` - - -* `type` {string} - -* Returns: {number} - -Node.js-specific extension to the `EventTarget` class that returns the number -of event listeners registered for the `type`. - -#### `nodeEventTarget.off(type, listener)` - - -* `type` {string} -* `listener` {Function|EventListener} - -* Returns: {EventTarget} this - -Node.js-speciic alias for `eventTarget.removeListener()`. - -#### `nodeEventTarget.on(type, listener[, options])` - - -* `type` {string} -* `listener` {Function|EventListener} -* `options` {Object} - * `once` {boolean} - -* Returns: {EventTarget} this - -Node.js-specific alias for `eventTarget.addListener()`. - -#### `nodeEventTarget.once(type, listener[, options])` - - -* `type` {string} -* `listener` {Function|EventListener} -* `options` {Object} - -* Returns: {EventTarget} this - -Node.js-specific extension to the `EventTarget` class that adds a `once` -listener for the given event `type`. This is equivalent to calling `on` -with the `once` option set to `true`. - -#### `nodeEventTarget.removeAllListeners([type])` - - -* `type` {string} - -Node.js-specific extension to the `EventTarget` class. If `type` is specified, -removes all registered listeners for `type`, otherwise removes all registered -listeners. - -#### `nodeEventTarget.removeListener(type, listener)` - - -* `type` {string} -* `listener` {Function|EventListener} - -* Returns: {EventTarget} this - -Node.js-specific extension to the `EventTarget` class that removes the -`listener` for the given `type`. The only difference between `removeListener()` -and `removeEventListener()` is that `removeListener()` will return a reference -to the `EventTarget`. - [WHATWG-EventTarget]: https://dom.spec.whatwg.org/#interface-eventtarget [`--trace-warnings`]: cli.html#cli_trace_warnings [`EventEmitter.defaultMaxListeners`]: #events_eventemitter_defaultmaxlisteners diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index 763542eb0fc2fa..4dd0b1ce71bed1 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -1,24 +1,19 @@ 'use strict'; const { - ArrayFrom, - Error, Map, Object, Set, Symbol, - NumberIsNaN, } = primordials; const { codes: { ERR_INVALID_ARG_TYPE, ERR_EVENT_RECURSION, - ERR_OUT_OF_RANGE, } } = require('internal/errors'); -const perf_hooks = require('perf_hooks'); const { customInspectSymbol } = require('internal/util'); const { inspect } = require('util'); @@ -29,11 +24,20 @@ const kTarget = Symbol('kTarget'); const kNewListener = Symbol('kNewListener'); const kRemoveListener = Symbol('kRemoveListener'); +let perf_hooks; + +function lazyNow() { + if (perf_hooks === undefined) + perf_hooks = require('perf_hooks'); + return perf_hooks.performance.now(); +} + + class Event { #type = undefined; #defaultPrevented = false; #cancelable = false; - #timestamp = perf_hooks.performance.now(); + #timestamp = lazyNow(); // Neither of these are currently used in the Node.js implementation // of EventTarget because there is no concept of bubbling or @@ -297,101 +301,6 @@ Object.defineProperties(EventTarget.prototype, { dispatchEvent: { enumerable: true } }); -class NodeEventTarget extends EventTarget { - static defaultMaxListeners = 10; - - #maxListeners = NodeEventTarget.defaultMaxListeners; - #maxListenersWarned = false; - - [kNewListener](size, type, listener, once, capture, passive) { - if (this.#maxListeners > 0 && - size > this.#maxListeners && - !this.#maxListenersWarned) { - this.#maxListenersWarned = true; - // No error code for this since it is a Warning - // eslint-disable-next-line no-restricted-syntax - const w = new Error('Possible EventTarget memory leak detected. ' + - `${size} ${type} listeners ` + - `added to ${inspect(this, { depth: -1 })}. Use ` + - 'setMaxListeners() to increase limit'); - w.name = 'MaxListenersExceededWarning'; - w.target = this; - w.type = type; - w.count = size; - process.emitWarning(w); - } - } - - setMaxListeners(n) { - if (typeof n !== 'number' || n < 0 || NumberIsNaN(n)) { - throw new ERR_OUT_OF_RANGE('n', 'a non-negative number', n); - } - this.#maxListeners = n; - return this; - } - - getMaxListeners() { - return this.#maxListeners; - } - - eventNames() { - return ArrayFrom(this[kEvents].keys()); - } - - listenerCount(type) { - const root = this[kEvents].get(String(type)); - return root !== undefined ? root.size : 0; - } - - off(type, listener, options) { - this.removeEventListener(type, listener, options); - return this; - } - - removeListener(type, listener, options) { - this.removeEventListener(type, listener, options); - return this; - } - - on(type, listener) { - this.addEventListener(type, listener); - return this; - } - - addListener(type, listener) { - this.addEventListener(type, listener); - return this; - } - - once(type, listener) { - this.addEventListener(type, listener, { once: true }); - return this; - } - - removeAllListeners(type) { - if (type !== undefined) { - this[kEvents].delete(String(type)); - } else { - this[kEvents].clear(); - } - } -} - -Object.defineProperties(NodeEventTarget.prototype, { - setMaxListeners: { enumerable: true }, - getMaxListeners: { enumerable: true }, - eventNames: { enumerable: true }, - listenerCount: { enumerable: true }, - off: { enumerable: true }, - removeListener: { enumerable: true }, - on: { enumerable: true }, - addListener: { enumerable: true }, - once: { enumerable: true }, - removeAllListeners: { enumerable: true }, -}); - -// EventTarget API - function validateListener(listener) { if (typeof listener === 'function' || (listener != null && @@ -432,10 +341,7 @@ function emitUnhandledRejectionOrErr(that, err, event) { process.emit('error', err, event); } -// EventEmitter-ish API: - module.exports = { Event, EventTarget, - NodeEventTarget, }; diff --git a/test/parallel/test-eventtarget.js b/test/parallel/test-eventtarget.js index 99d717abda6e8d..3e0f9b0f54ae33 100644 --- a/test/parallel/test-eventtarget.js +++ b/test/parallel/test-eventtarget.js @@ -5,7 +5,6 @@ const common = require('../common'); const { Event, EventTarget, - NodeEventTarget, } = require('internal/event_target'); const { @@ -50,10 +49,12 @@ ok(EventTarget); ev.preventDefault(); strictEqual(ev.defaultPrevented, true); } + { const ev = new Event('foo'); deepStrictEqual(Object.keys(ev), ['isTrusted']); } + { const eventTarget = new EventTarget(); @@ -78,6 +79,7 @@ ok(EventTarget); eventTarget.removeEventListener('foo', ev1); eventTarget.dispatchEvent(new Event('foo')); } + { // event subclassing const SubEvent = class extends Event {}; @@ -87,35 +89,6 @@ ok(EventTarget); eventTarget.addEventListener('foo', fn, { once: true }); eventTarget.dispatchEvent(ev); } -{ - const eventTarget = new NodeEventTarget(); - strictEqual(eventTarget.listenerCount('foo'), 0); - deepStrictEqual(eventTarget.eventNames(), []); - - const ev1 = common.mustCall(function(event) { - strictEqual(event.type, 'foo'); - strictEqual(this, eventTarget); - }, 2); - - const ev2 = { - handleEvent: common.mustCall(function(event) { - strictEqual(event.type, 'foo'); - strictEqual(this, ev2); - }) - }; - - eventTarget.addEventListener('foo', ev1); - eventTarget.addEventListener('foo', ev2, { once: true }); - strictEqual(eventTarget.listenerCount('foo'), 2); - ok(eventTarget.dispatchEvent(new Event('foo'))); - strictEqual(eventTarget.listenerCount('foo'), 1); - eventTarget.dispatchEvent(new Event('foo')); - - eventTarget.removeEventListener('foo', ev1); - strictEqual(eventTarget.listenerCount('foo'), 0); - eventTarget.dispatchEvent(new Event('foo')); -} - { const eventTarget = new EventTarget(); @@ -124,89 +97,6 @@ ok(EventTarget); ok(!eventTarget.dispatchEvent(event)); } -{ - const eventTarget = new NodeEventTarget(); - strictEqual(eventTarget.listenerCount('foo'), 0); - deepStrictEqual(eventTarget.eventNames(), []); - - const ev1 = common.mustCall((event) => { - strictEqual(event.type, 'foo'); - }, 2); - - const ev2 = { - handleEvent: common.mustCall((event) => { - strictEqual(event.type, 'foo'); - }) - }; - - strictEqual(eventTarget.on('foo', ev1), eventTarget); - strictEqual(eventTarget.once('foo', ev2, { once: true }), eventTarget); - strictEqual(eventTarget.listenerCount('foo'), 2); - eventTarget.dispatchEvent(new Event('foo')); - strictEqual(eventTarget.listenerCount('foo'), 1); - eventTarget.dispatchEvent(new Event('foo')); - - strictEqual(eventTarget.off('foo', ev1), eventTarget); - strictEqual(eventTarget.listenerCount('foo'), 0); - eventTarget.dispatchEvent(new Event('foo')); -} - -{ - const eventTarget = new NodeEventTarget(); - strictEqual(eventTarget.listenerCount('foo'), 0); - deepStrictEqual(eventTarget.eventNames(), []); - - const ev1 = common.mustCall((event) => { - strictEqual(event.type, 'foo'); - }, 2); - - const ev2 = { - handleEvent: common.mustCall((event) => { - strictEqual(event.type, 'foo'); - }) - }; - - eventTarget.addListener('foo', ev1); - eventTarget.once('foo', ev2, { once: true }); - strictEqual(eventTarget.listenerCount('foo'), 2); - eventTarget.dispatchEvent(new Event('foo')); - strictEqual(eventTarget.listenerCount('foo'), 1); - eventTarget.dispatchEvent(new Event('foo')); - - eventTarget.removeListener('foo', ev1); - strictEqual(eventTarget.listenerCount('foo'), 0); - eventTarget.dispatchEvent(new Event('foo')); -} - -{ - const eventTarget = new NodeEventTarget(); - strictEqual(eventTarget.listenerCount('foo'), 0); - deepStrictEqual(eventTarget.eventNames(), []); - - // Won't actually be called. - const ev1 = () => {}; - - // Won't actually be called. - const ev2 = { handleEvent() {} }; - - eventTarget.addListener('foo', ev1); - eventTarget.addEventListener('foo', ev1); - eventTarget.once('foo', ev2, { once: true }); - eventTarget.once('foo', ev2, { once: false }); - eventTarget.on('bar', ev1); - strictEqual(eventTarget.listenerCount('foo'), 2); - strictEqual(eventTarget.listenerCount('bar'), 1); - deepStrictEqual(eventTarget.eventNames(), ['foo', 'bar']); - eventTarget.removeAllListeners('foo'); - strictEqual(eventTarget.listenerCount('foo'), 0); - strictEqual(eventTarget.listenerCount('bar'), 1); - deepStrictEqual(eventTarget.eventNames(), ['bar']); - eventTarget.removeAllListeners(); - strictEqual(eventTarget.listenerCount('foo'), 0); - strictEqual(eventTarget.listenerCount('bar'), 0); - deepStrictEqual(eventTarget.eventNames(), []); -} - { const uncaughtException = common.mustCall((err, event) => { strictEqual(err.message, 'boom'); @@ -300,25 +190,6 @@ ok(EventTarget); target.dispatchEvent(new Event('foo')); } -{ - const target = new NodeEventTarget(); - - process.on('warning', common.mustCall((warning) => { - ok(warning instanceof Error); - strictEqual(warning.name, 'MaxListenersExceededWarning'); - strictEqual(warning.target, target); - strictEqual(warning.count, 2); - strictEqual(warning.type, 'foo'); - ok(warning.message.includes( - '2 foo listeners added to NodeEventTarget')); - })); - - strictEqual(target.getMaxListeners(), NodeEventTarget.defaultMaxListeners); - target.setMaxListeners(1); - target.on('foo', () => {}); - target.on('foo', () => {}); -} - { const target = new EventTarget(); const event = new Event('foo');