-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
Add AbortSignal.abort() static method (FF88) #3616
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hamishwillee Sounds plausible to me, from reading the background docs, but I'm not really sure I get it.
Therefore, paging @annevk , as he usually understands this kind of stuff way better than me. Anne, do you have time to give this content addition a very quick look over to see if it makes sense?
@annevk When you have a moment, can you please sanity check as per #3616 (review) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description seems somewhat wrong. This is a shorthand for:
const controller = new AbortController();
controller.abort();
return controller.signal;
The signal is something you pass to fetch()
and other APIs, it's not something that is created there.
@annevk Thanks, you're right. I understand what this shorthand does, but not what scenario you would ever need to use it in. My understand is normally you create and pass in the signal to your fetch. As some point later you decide to abort (in this case following a timeout, but maybe following a button press). const controller = new AbortController();
const signal = controller.signal;
setTimeout(() => controller.abort(), 5000);
fetch(url, { signal }).then(response => {
return response.text();
}).then(text => {
console.log(text);
}); Can you give me a real world example of when I might need to pass in an already cancelled signal to a fetch, ideally with a code fragment? Something along the lines of "I'm using fetch to get a resource, but X happens, so I need to pass in cancelled signal. The various discussions on this had some info (which I paraphrased as below). That kind of explains why you might want to do this, but I don't really "get" the scenario that would make this useful. |
This would be more for a utility function that gives you the state of an operation as a signal. If the operation was already aborted, it would use this and not setup more complicated tracking. (The context was about code in Node.js, not directly related to |
@annevk Thanks, but whether it is used in a browser or node, it still isn't clear why you would ever need to pass an aborted signal for something that is already aborted? So unless someone can provide a concrete example/reason for this, all I can do is what you said - which is say that it is a shorthand/utility function. Then wait for unanswerable questions on why we bothered :-0 |
I think it depends on how you organize your code, but if you already have logic to deal with an aborted fetch and you want to trigger that logic if another operation got aborted (from which you obtained a signal), it might well make sense to pass the aborted signal on to the fetch, which will then trigger the aborted fetch logic. |
18079d5
to
2244fbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this! This looks good to me.
Thanks @annevk - appreciate your patience! @chrisdavidmills I think this is good to go. It isn't particularly "rich content" but I think now enough for anyone who has constructed their code in such a way that this is useful, to understand how it can be used. |
Thanks for the help @annevk , and cheers for the perseverence @hamishwillee . This was a tricky one, and I think you've handled it well. |
Firefox 88 adds a new static method
AbortSignal.abort()
.BCD and Release notes in separate PR.
FYI @sideshowbarker
MDN tracking bug: #3460