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

Implement abort #547

Closed
lukeapage opened this issue Jul 26, 2017 · 6 comments · Fixed by #592
Closed

Implement abort #547

lukeapage opened this issue Jul 26, 2017 · 6 comments · Fixed by #592

Comments

@lukeapage
Copy link

As per whatwg/fetch#523

I guess it should wait till the above is merged.

@jimmywarting
Copy link

jimmywarting commented Aug 9, 2017

just a reference: https://github.com/mo/abortcontroller-polyfill/blob/master/src/abortcontroller.js#L8-L47
this will patch fetch and return a rejection instead, it dosen't abort the fetch

@coldhamix
Copy link

coldhamix commented Sep 21, 2017

@lukeapage @mislav whatwg/fetch#523 just got merged

@lukeapage
Copy link
Author

Yep, I've been following. Good news.

@mislav
Copy link
Contributor

mislav commented Sep 24, 2017

I quite dig the polyfill that @jimmywarting shared. Does anyone think that this project should include that kind of polyfill? Should we try to patch native window.fetch if it doesn't support aborting?

@jimmywarting
Copy link

You should try calling xhr.abort to not wast bandwidth

@jamesplease
Copy link
Contributor

jamesplease commented Dec 7, 2017

I agree with @mislav 's and @jimmywarting 's suggestions over in #572 that keeping the AbortController and AbortSignal polyfills separate from this library seems preferable.

Should we try to patch native window.fetch if it doesn't support aborting?

I think so. You may already be thinking this, but it would probably be a breaking change to the API, as this library would start polyfilling many more apps than it currently does. This might warrant a major version bump if this lib follows semver.

You should try calling xhr.abort to not wast bandwidth

It would be pretty cool to see this library update to accept a (duck-typed) AbortSignal and call xhr.abort whenever the abort event is dispatched.

A quick glance at the code suggests to me that this should be straightforward to implement. Perhaps the hardest part will be updating the feature detection code.

I wrote a quick snippet that seems like a step in the right direction. I tested it in Safari, Chrome, and Firefox, and got the expected results (no, no, yes, respectively).

What do folks think? Should I code up a PR that implements something along these lines? PR'd at #592

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2020
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 a pull request may close this issue.

6 participants
@mislav @lukeapage @jimmywarting @jamesplease @coldhamix and others