-
Notifications
You must be signed in to change notification settings - Fork 295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow subclassing of AbortController / AbortSignal #1162
Comments
I think the only reason I guess what you need is that when you instantiate a subclass of And then perhaps Overall though I think your rationale is sound and I think people generally appreciated being able to do this with |
I've continued to experiment and did manage to successfully get them subclassing, but it's a bit of a pain to get working, and doesn't allow for class fields or constructor initialisation logic in the signal: class WatchSignal extends AbortSignal {
get watching() {
return !this.aborted
}
}
class WatchController extends AbortController {
constructor() {
super()
// Here is the magic line:
Object.setPrototypeOf(this.signal, WatchSignal.prototype)
}
}
const wc = new WatchController()
console.assert( wc.signal.watching ) Doing it this way also means class fields wont be assigned and private fields won't work.
I like the pattern but I struggle to see how it could apply to this considering we have two discrete objects which share a private API: class WatchSignal extends AbortSignal {
constructor(abort) {
super(abort)
}
}
class WatchController extends AbortController {
constructor() {
super(function (abort) { // What is even `abort`? Maybe userland doesn't care?
// Presumably we need to return an AbortSignal instance
// to have AbortController assign this as the signal?
return new WatchSignal(abort)
})
}
} |
I'm not entirely sure about that part either, but something like that. @domenic used to think about this problem space a fair bit and might have ideas. |
@jakearchibald also had some insight about 2 years ago, in this thread: #981 (comment). |
Return override permits this. class WatchSignal extends function(o) { return Object.setPrototypeOf(o, new.target.prototype); } {
#fields = "here";
static {
Object.setPrototypeOf(this.prototype, AbortSignal.prototype);
}
}
class WatchController extends AbortController {
constructor() {
new WatchSignal(super().signal);
}
} (Note the use of new.target.prototype in the “identity class”: honoring new.target.prototype permits further subclassing of WatchSignal without needing to do this again.) Further refinements are possible if you want it to behave as closely as possible to a native platform interface object (really you need two classes to get it right, a “private” and “public” pair, so that the public constructor.[[Prototype]] becomes immaterial to what happens in the private constructor logic and no “implementation artifacts” are left hanging on the user-reachable outside surface), but things get increasingly complicated and “fussy” the deeper you go. Decorators will allow us to encapsulate a lot of that boilerplate much more neatly, but not exactly all of it. |
It would probably make sense if it were just the controller itself, this also allows a subclass pair to share data between the controller and signal easily: const activePriorities = new WeakMap();
class PrioritySignal extends AbortSignal {
#priorityController;
constructor(priorityController) {
// AbortSignal can access the usual internal slots of the controller
super(priorityController);
// We can access whatever shared things we need of the watchController here too
this.#priorityController = priorityController;
}
get priority() {
return activePriorities.get(this.#priorityController);
}
}
class PriorityController extends AbortController {
constructor(initialPriority = 1) {
super(controller => {
return new PrioritySignal(controller);
});
activePriorities.set(this, initialPriority);
}
} |
Currently it is not possible to subclass these, despite DOM interfaces doing so (
TaskController
/TaskSignal
). Currently each of the properties inAbortSignal
asserts that it is either anAbortSignal
orTaskSignal
, and if it is not it will throw. This means doing the following results in an error:Instead one has to use a delegate class which means re-implementing all methods and
Symbol.hasInstance
if you want to keep brand checks.Making
AbortController
extensible comes with its own problems, however. Chiefly how does one associate the controller with the signal?Why make it extensible?
Of course the request to make it extensible begs the question as to why. I think
TaskController
/TaskSignal
provides good precedent here - extended versions are useful to pack extra data into an interface, especially one that can be passed between contexts.TaskSignals
extend the concept to add apriority
andprioritychange
event, and here are some other ideas that could be implemented in userland:PauseController
/PauseSignal
with apaused
property andpausedchanged
event, allowing for pausing/resuming of tasks.RetryController
/RetrySignal
, with aretry
number property, allowing consumers to know how often to retry a task.DelayController
/DelaySignal
, which has awaitTime
number property, allowing consumers to delay a task before execution.All of these subclasses can still be passed freely to downstream functions like
fetch
, but avoid the need to send a whole slew of signals to a function.The text was updated successfully, but these errors were encountered: