Skip to content
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

Open
keithamus opened this issue Feb 15, 2023 · 6 comments
Open

Allow subclassing of AbortController / AbortSignal #1162

keithamus opened this issue Feb 15, 2023 · 6 comments
Labels
addition/proposal New features or enhancements needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan needs implementer interest Moving the issue forward requires implementers to express interest topic: aborting AbortController and AbortSignal

Comments

@keithamus
Copy link

Currently it is not possible to subclass these, despite DOM interfaces doing so (TaskController/TaskSignal). Currently each of the properties in AbortSignal asserts that it is either an AbortSignal or TaskSignal, and if it is not it will throw. This means doing the following results in an error:

class WatchSignal extends AbortSignal {}
new WatchSignal() // This throws... but we can try Object.create()

const ws = Object.create(WatchSignal.prototype) // We got signal!
console.assert( ws instanceof AbortSignal )

ws.aborted // throws with invocation or "does not implement interface AbortSignal" 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 a priority and prioritychange event, and here are some other ideas that could be implemented in userland:

  • PauseController/PauseSignal with a paused property and pausedchanged event, allowing for pausing/resuming of tasks.
  • RetryController/RetrySignal, with a retry number property, allowing consumers to know how often to retry a task.
  • DelayController/DelaySignal, which has a waitTime 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.

@annevk annevk added needs implementer interest Moving the issue forward requires implementers to express interest needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan topic: aborting AbortController and AbortSignal addition/proposal New features or enhancements labels Feb 15, 2023
@annevk
Copy link
Member

annevk commented Feb 15, 2023

I think the only reason new WatchSignal throws is because new AbortSignal throws. Generally you can subclass web platform objects.

I guess what you need is that when you instantiate a subclass of AbortController you need some kind of way to override the creation of an AbortSignal with another subclass. Perhaps using https://blog.domenic.me/the-revealing-constructor-pattern/?

And then perhaps AbortSignal.abort() and AbortSignal.timeout() need to be able to return a subclass when used on one. I think JavaScript has some capability for that as well, but I believe that feature is not much liked.

Overall though I think your rationale is sound and I think people generally appreciated being able to do this with EventTarget.

@keithamus
Copy link
Author

keithamus commented Feb 15, 2023

I think the only reason new WatchSignal throws is because new AbortSignal throws. Generally you can subclass web platform objects.

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.

And then perhaps AbortSignal.abort() and AbortSignal.timeout() need to be able to return a subclass when used on one. I think JavaScript has some capability for that as well, but I believe that feature is not much liked.

[Symbol.species] but I think that is recommended against and being proposed for removal.

Perhaps using https://blog.domenic.me/the-revealing-constructor-pattern/?

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)
    })
  } 
}

@annevk
Copy link
Member

annevk commented Feb 15, 2023

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.

@keithamus
Copy link
Author

@jakearchibald also had some insight about 2 years ago, in this thread: #981 (comment).

@bathos
Copy link

bathos commented Feb 15, 2023

doesn't allow for class fields or constructor initialisation logic in the signal

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.

@Jamesernator
Copy link

Jamesernator commented Jul 20, 2023

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)
    })
  } 
}

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);
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs concrete proposal Moving the issue forward requires someone to figure out a detailed plan needs implementer interest Moving the issue forward requires implementers to express interest topic: aborting AbortController and AbortSignal
Development

No branches or pull requests

4 participants