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

Add AbortSignal.abort() static method (FF88) #3616

Merged
merged 9 commits into from
Apr 16, 2021

Conversation

hamishwillee
Copy link
Collaborator

Firefox 88 adds a new static method AbortSignal.abort().

BCD and Release notes in separate PR.

FYI @sideshowbarker

MDN tracking bug: #3460

@hamishwillee hamishwillee requested a review from a team as a code owner March 29, 2021 03:58
@hamishwillee hamishwillee requested review from wbamberg and removed request for a team March 29, 2021 03:58
Copy link
Contributor

@chrisdavidmills chrisdavidmills left a 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?

@hamishwillee
Copy link
Collaborator Author

@annevk When you have a moment, can you please sanity check as per #3616 (review)

Copy link
Contributor

@annevk annevk left a 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.

@hamishwillee
Copy link
Collaborator Author

hamishwillee commented Apr 13, 2021

@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.

image

@annevk
Copy link
Contributor

annevk commented Apr 13, 2021

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 fetch() I think.)

@hamishwillee
Copy link
Collaborator Author

@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

@annevk
Copy link
Contributor

annevk commented Apr 13, 2021

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.

@hamishwillee hamishwillee requested a review from a team as a code owner April 16, 2021 07:10
Copy link
Contributor

@annevk annevk left a 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.

@hamishwillee
Copy link
Collaborator Author

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.

@chrisdavidmills
Copy link
Contributor

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.

@chrisdavidmills chrisdavidmills merged commit 1d4b8bc into mdn:main Apr 16, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants