Events or messages?
Is this a messaging system or an event system. As I see it, it is a messaging system.
Assuming this is for the browser you may prefer the provided events.Event API which will use the event stack. NOTE check comparability before using.
Review
A general style code review. Code style is subjective and thus these points are just suggestions
When possible use instanceof
rather than typeof
as the former has considerably less overhead
For example
if (typeof handler === 'function') {
can be
if (handler instanceof Function) {
Do not silence errors (oh I mean... "NEVER!! silence errors"). The above point (vetting the callback) will silence code that is passing the wrong type of object. Silencing means that an error can go unnoticed during development.
Either throw when vetting or let JS throw naturally.
Always delimit code blocks with {}
Eg if (foo) return
should be if (foo) { return }
Use semicolons!
Use constants whenever possible. You have let set = this.get(type)
should be const set = ...
Avoid redundant clauses
You have the statement
if (set?.delete(handler) && set?.size === 0) {
The second ?.
is redundant. If the first clause is true then set
must not be undefined
.
Also use logical not !
rather than === 0
. Map.size
will only ever be a Number
thus !set.size
will always be true
for 0
if (set?.delete(handler) && !set.size) {
Avoid intermediate calls, especially when using rest parameters as this forces array creation overhead just to pass arguments between functions.
You have
class HandlerSet extends Map {
forEach(...args) {
[...this.values()].forEach(...args);
}
class EventSystem extends Map {
emit(type, ...args) {
this.get(type)?.forEach(handler => handler(...args));
}
Can be...
emit(type, ...args) {
const handlers = this.get(type);
if (handlers) {
for (const handler of handlers.values()) { handler(...args) }
}
}
...reducing memory and iteration overheads. Also removes the need for HandlerSet.forEach
Be careful with naming.
Avoid naming variables for their type. Name variables for what they represent, not what they are.
I find your name selection is obfuscating the functionality of your code. The main problem is that the concepts of a listener and an event have been blurred. An event has a name eg a load event would be called "load"
. Each named event has a set of listeners / callbacks (functions)
Notes on some of the names you used
handler
Conventional JS uses listeners rather than handlers, however in this case they are much more like callbacks, listener
or callback
would be more appropreate (NOTE callback
as one word not callBack
).
HandlerSet
is extending a Map
. Names should avoid including type descriptions. Adding a name of a similar type is just confusing.
EventSystem
is it really a system? Don't add superfluous content to names. Maybe GlobalEvents
would be better?
EventSystem.on
ambiguous name, maybe addListener
, addCallback
, or even just addCall
?
EventSystem.once
Similar to on
. Personally the once
functionality is inappropriate or can be handled using options when adding a listener.
EventSystem.emit
JS convention is that events are fired not emitted thus maybe fire
would be better
EventSystem.delete
Delete what, an event or a listener? Names should never be ambiguous.
Rewrite?... No.
I did rewrite your code but I could not reconcile its usefulness due to ambiguity of (guessed) usage cases.
Thus what follows is an Example and is just a modification of code I use to add a messaging.
Example
Note the example code has been parsed but is untested.
Note the Map
named events
is closed over and thus protected from miss use.
Note This does not check if the listener is a function. Doing so can hide/silence bad code. If the cb is not a function it will throw in fireEvent
// Naming: cb and cbs short for "callback" (a function) and "callbacks" (array of callback objects)
function Events(owner = {}) {
const events = new Map(); // named arrays of listener objects
return Object.assign(owner, {
addListener(name, cb, options = {}) {
var cbs;
((events.get(name) ?? (events.set(name, cbs = []), cbs)).push({cb, options}));
},
removeEvent(name) { events.delete(name) },
removeListener(name, cb) {
const cbs = events.get(name);
if (cbs?.length) {
const idx = cbs.findIdx(listener => listener.cb === cb);
idx > -1 && cbs.splice(idx, 1);
}
},
fireEvent(name, data = {}) {
var i = 0;
const cbs = events.get(name);
if (cbs?.length) {
for (const {cb, options} of cbs) {
options.once && cbs.splice(i--, 1);
cb.bind(owner)({type: name, target: owner, ...data});
i++;
}
}
},
});
}
export {Events};