Add support for AbortController in EventEmitter
Categories
(DevTools :: Shared Components, task)
Tracking
(firefox92 fixed)
Tracking | Status | |
---|---|---|
firefox92 | --- | fixed |
People
(Reporter: nchevobbe, Assigned: nchevobbe)
Details
Attachments
(1 file)
AbortController allows to remove regular event listeners (see signal
parameter in https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#parameters)
It's super useful to be able to remove multiple event listeners at once.
As DevTools code is using a mixture of regular event listeners and EventEmitter listeners, it would be great to bring such capability to EventEmitter as well so we'd have only one AbortController signal handle all our event listeners.
So let's say we have this use case:
const RandomDevToolsClass {
constructor(window, obj) {
this.window = window;
this.obj = obj;
this.window.addEventListener("load", this._onLoad)
this.obj.on("refresh", this._onRefresh)
}
destroy() {
this.window.removeEventListener("load", this._onLoad)
this.obj.off("refresh", this._onRefresh)
}
}
Here, we have to:
- set
window
andobj
onthis
, so we can remove the event listener later - have
_onLoad
and_onRefresh
as methods (and not inline anonymous function), so we can remove the event listener later - in
destroy
go over all the event listeners we set to remove them
With AbortController, it could be transformed to
const RandomDevToolsClass {
constructor(window, obj) {
this.abortController = new AbortController();
const {signal} = this.abortController;
window.addEventListener("load", () => { /* … */}, {signal})
obj.on("refresh", () => { /* … */}, {signal})
}
destroy() {
this.abortController.abort();
}
}
of course, we'll still have cases where we need to set window
and obj
onto this
, and other cases where we still want the event listener callbacks to not be inline anonymous functions, but at least with AbortController this is giving us the ability to choose.
Comment 1•3 years ago
|
||
I'm wondering if we could make our common base classes inherit from AbortSignal/AbortController, so that we don't have to create an AbortController, just to fetch it signal and avoid having to abort the abort controller...
So that we can do:
const RandomDevToolsClass extends AbortController {
constructor(window, obj) {
const signal = this.signal;
window.addEventListener("load", () => { /* … */}, {signal})
obj.on("refresh", () => { /* … */}, {signal})
}
destroy() {
// This will call AbortController.destroy()
super.destroy()
}
}
or
const RandomDevToolsClass extends AbortSignal {
constructor(window, obj) {
const signal = this;
window.addEventListener("load", () => { /* … */}, {signal})
obj.on("refresh", () => { /* … */}, {signal})
}
destroy() {
// Notify that we should unregister any async listener via AbortSignal interface
this.dispatchEvent(new Event("abort"));
}
}
or
const RandomDevToolsClass extends EventTarget {
constructor(window, obj) {
this.aborted = fase;
const signal = this;
window.addEventListener("load", () => { /* … */}, {signal})
obj.on("refresh", () => { /* … */}, {signal})
}
destroy() {
// Implement AbortSignal interface
this.aborted = true;
this.dispatchEvent(new Event("abort"));
}
}
As many classes inherit from EventEmitter, and, inheriting from many classes is complex... may be we should make EventEmitter expose an AbortSignal somehow.
Comment 2•3 years ago
|
||
Once we do something in this bug, we can probably revisit this comment and use a signal for this:
https://phabricator.services.mozilla.com/D114367#3720664
Assignee | ||
Comment 3•3 years ago
|
||
Updated•3 years ago
|
Comment 5•3 years ago
|
||
bugherder |
Description
•