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

Proposal: AbortSignal.none() #1277

Open
nzakas opened this issue Apr 9, 2024 · 4 comments
Open

Proposal: AbortSignal.none() #1277

nzakas opened this issue Apr 9, 2024 · 4 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: aborting AbortController and AbortSignal

Comments

@nzakas
Copy link

nzakas commented Apr 9, 2024

What problem are you trying to solve?

When writing a function that accepts an AbortSignal, I typically write a function that looks like this:

async function doSomething(arg1, arg2, { signal } = {}) {
   // body
}

Because signal is optional, I'd like a way to specify a default value that can be used throughout the function to indicate "this function is not aborted"

What solutions exist today?

There are two solutions I've used so far, and both have proven error prone:

async function doSomething(arg1, arg2, { signal } = {}) {
   // solution 1: check for signal constantly
   if (signal) {
     signal.throwIfAborted();
   }

   // solution 2: optional chaining
  signal?.throwIfAborted()
}

In both cases, the issue I've faced is that it's easy to forget to check if signal has been passed and so errors tend to show up later.

How would you solve it?

I'd create a new AbortSignal.none() method that would return an AbortSignal that is not aborted and never will be. That way, I could write something like this:

async function doSomething(arg1, arg2, { signal = AbortSignal.none() } = {}) {
   
    // no need to check anything
    signal.throwIfAborted();
}

Anything else?

Thank you for reading. 😄

@nzakas nzakas added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest labels Apr 9, 2024
@jasnell
Copy link
Contributor

jasnell commented Apr 9, 2024

I understand the case you're going for but I'm not sure there's enough value relative to just passing undefined and using optional chaining.

@keithamus
Copy link

One way to create such a signal is new AbortController().signal but that does create an object only to have to clean it up.

new AbortSignal() throws. In theory that could create a never-aborting signal. Relatedly #1162 discusses the ideas of subclassing signals, which would likely mean lifting the exception from AbortSignal.

@nzakas
Copy link
Author

nzakas commented Apr 9, 2024

@jasnell yeah, that's the approach I've been taking, but again, it's easy to forget to add the ? and discover a bug later on as a result.

In my mind, an AbortSignal.none() is the companion to AbortSignal.abort() in the same way that Promise.resolve() is the companion to Promise.reject().

@keithamus oh yes, it's definitely possible to do new AbortController().signal, that's a good call. 👍

@jasnell
Copy link
Contributor

jasnell commented Apr 10, 2024

Yes, I get it, unfortunately APIs are going to have to still deal with the likelihood that signal is passed as undefined, so I'm not sure the new API provides enough of a distinct benefit

@annevk annevk added the topic: aborting AbortController and AbortSignal label Apr 10, 2024
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 implementer interest Moving the issue forward requires implementers to express interest topic: aborting AbortController and AbortSignal
Development

No branches or pull requests

4 participants