Skip to content

Commit 7dd9aff

Browse files
committed
fix(core events): Fix references in add_event_listener and remove_event_listener.
add_event_listener and remove_event_listener did incorrectly use an object instead of a map to store element references. This is now fixed.
1 parent c2caf48 commit 7dd9aff

File tree

2 files changed

+32
-32
lines changed

2 files changed

+32
-32
lines changed

src/core/events.js

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// Event listener registration for easy-to-remove event listeners.
44
// once Safari supports the ``signal`` option for addEventListener we can abort
55
// event handlers by calling AbortController.abort().
6-
export const event_listener_map = {};
6+
export const event_listener_map = new Map();
77

88
/**
99
* Add an event listener to a DOM element under a unique id.
@@ -23,19 +23,21 @@ const add_event_listener = (el, event_type, id, cb, opts = {}) => {
2323
remove_event_listener(el, id); // do not register one listener twice.
2424

2525
// Create event_listener_map entry if not existent.
26-
if (!event_listener_map[el]) {
27-
event_listener_map[el] = {};
26+
if (!event_listener_map.has(el)) {
27+
event_listener_map.set(el, new Map());
2828
}
2929
let _cb = cb;
3030
if (opts?.once === true) {
3131
// For `once` events, also remove the entry from the event_listener_map.
3232
_cb = (e) => {
33-
delete event_listener_map[el][id];
33+
event_listener_map.get(el)?.delete(id);
3434
cb(e);
3535
};
3636
}
3737
// Only `capture` option is necessary for `removeEventListener`.
38-
event_listener_map[el][id] = [event_type, _cb, opts.capture ? opts : undefined];
38+
event_listener_map
39+
.get(el)
40+
.set(id, [event_type, _cb, opts.capture ? opts : undefined]);
3941
el.addEventListener(event_type, _cb, opts);
4042
};
4143

@@ -50,26 +52,26 @@ const remove_event_listener = (el, id) => {
5052
if (!el?.removeEventListener) {
5153
return; // nothing to do.
5254
}
53-
const el_events = event_listener_map[el];
55+
const el_events = event_listener_map.get(el);
5456
if (!el_events) {
5557
return;
5658
}
5759
let entries;
5860
if (id) {
5961
// remove event listener with specific id
60-
const entry = el_events[id];
62+
const entry = el_events.get(id);
6163
entries = entry ? [[id, entry]] : [];
6264
} else {
6365
// remove all event listeners of element
64-
entries = Object.entries(el_events);
66+
entries = el_events.entries();
6567
}
6668
for (const entry of entries || []) {
6769
el.removeEventListener(entry[1][0], entry[1][1], entry[1][2]);
6870
// Delete entry from event_listener_map
69-
delete event_listener_map[el][entry[0]];
71+
event_listener_map.get(el).delete(entry[0]);
7072
// Delete element from event_listener_map if no more events are registered.
71-
if (!Object.keys(event_listener_map[el]).length) {
72-
delete event_listener_map[el];
73+
if (!event_listener_map.get(el).size) {
74+
event_listener_map.delete(el);
7375
}
7476
}
7577
};

src/core/events.test.js

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ describe("core.events tests", () => {
77
describe("1 - add / remove event listener", () => {
88
afterEach(() => {
99
// Clear event_listener_map after each test.
10-
for (const el in event_listener_map) {
11-
delete event_listener_map[el];
12-
}
10+
event_listener_map.clear();
1311
});
1412

1513
it("Registers events only once and unregisters events.", (done) => {
@@ -66,10 +64,10 @@ describe("core.events tests", () => {
6664
once: true,
6765
});
6866

69-
expect(event_listener_map[el].test_once_event).toBeDefined();
67+
expect(event_listener_map.get(el).get("test_once_event")).toBeDefined();
7068
el.dispatchEvent(new Event("test"));
7169

72-
expect(event_listener_map[el].test_once_event).not.toBeDefined();
70+
expect(event_listener_map.get(el).get("test_once_event")).not.toBeDefined();
7371
});
7472

7573
it("Removes a specific event listener.", () => {
@@ -79,7 +77,7 @@ describe("core.events tests", () => {
7977

8078
// register the once-event handler
8179
events.add_event_listener(el, "test", "test_event", () => cnt++);
82-
expect(event_listener_map[el].test_event).toBeDefined();
80+
expect(event_listener_map.get(el).get("test_event")).toBeDefined();
8381

8482
el.dispatchEvent(new Event("test"));
8583
expect(cnt).toBe(1);
@@ -90,9 +88,9 @@ describe("core.events tests", () => {
9088
events.remove_event_listener(el, "test_event");
9189

9290
// Now the event listener should be removed.
93-
expect(event_listener_map[el]?.test_once_event).not.toBeDefined();
91+
expect(event_listener_map.get(el)?.get("test_once_event")).not.toBeDefined();
9492
// Even the element itself should be removed, if there are no more event listeners on it.
95-
expect(event_listener_map[el]).not.toBeDefined();
93+
expect(event_listener_map.get("el")).not.toBeDefined();
9694

9795
// counter should not increase anymore
9896
el.dispatchEvent(new Event("test"));
@@ -101,7 +99,7 @@ describe("core.events tests", () => {
10199

102100
it("Remove single and all event listeners from an element, not touching others.", () => {
103101
const el1 = document.createElement("div");
104-
const el2 = document.createElement("span");
102+
const el2 = document.createElement("div");
105103

106104
let cnt1 = 0;
107105
let cnt2 = 0;
@@ -117,12 +115,12 @@ describe("core.events tests", () => {
117115
events.add_event_listener(el1, "test3", "test_event_3", () => cnt2++);
118116
events.add_event_listener(el2, "test4", "test_event_4", () => cnt3++);
119117

120-
expect(event_listener_map[el1].test_event_1).toBeDefined();
121-
expect(event_listener_map[el1].test_event_2).toBeDefined();
122-
expect(event_listener_map[el1].test_event_3).toBeDefined();
123-
expect(event_listener_map[el2].test_event_4).toBeDefined();
118+
expect(event_listener_map.get(el1).get("test_event_1")).toBeDefined();
119+
expect(event_listener_map.get(el1).get("test_event_2")).toBeDefined();
120+
expect(event_listener_map.get(el1).get("test_event_3")).toBeDefined();
121+
expect(event_listener_map.get(el2).get("test_event_4")).toBeDefined();
124122

125-
expect(Object.keys(event_listener_map).length).toBe(2);
123+
expect(event_listener_map.size).toBe(2);
126124

127125
el1.dispatchEvent(new Event("test1"));
128126
expect(cnt1).toBe(1);
@@ -151,11 +149,11 @@ describe("core.events tests", () => {
151149

152150
// Remove only test_event_1
153151
events.remove_event_listener(el1, "test_event_1");
154-
expect(event_listener_map[el1].test_event_1).not.toBeDefined();
155-
expect(event_listener_map[el1].test_event_2).toBeDefined();
156-
expect(event_listener_map[el1].test_event_3).toBeDefined();
157-
expect(event_listener_map[el2].test_event_4).toBeDefined();
158-
expect(Object.keys(event_listener_map).length).toBe(2);
152+
expect(event_listener_map.get(el1).get("test_event_1")).not.toBeDefined();
153+
expect(event_listener_map.get(el1).get("test_event_2")).toBeDefined();
154+
expect(event_listener_map.get(el1).get("test_event_3")).toBeDefined();
155+
expect(event_listener_map.get(el2).get("test_event_4")).toBeDefined();
156+
expect(event_listener_map.size).toBe(2);
159157

160158
// Counter should not increase anymore on event "test1"
161159
el1.dispatchEvent(new Event("test1"));
@@ -173,8 +171,8 @@ describe("core.events tests", () => {
173171

174172
// Remove all event handler on el1
175173
events.remove_event_listener(el1);
176-
expect(event_listener_map[el1]).not.toBeDefined();
177-
expect(Object.keys(event_listener_map).length).toBe(1);
174+
expect(event_listener_map.get(el1)).not.toBeDefined();
175+
expect(event_listener_map.size).toBe(1);
178176

179177
// Counter should not increase anymore on el1
180178
el1.dispatchEvent(new Event("test1"));

0 commit comments

Comments
 (0)