I'm not sure this failed because of my PR? Test seems unrelated?

Right, I see PR#67 resolves the failing tests

@stereobooster I'm kinda wondering if that piece is required to make this PR usable TBH. I love the idea of keeping things split up, but we should probably just make a call on either supporting or not supporting abort. If we say it's important enough, probably best to install that polyfill (though it pains me).

@developit yes it is required, if browser doesn't providefetch, it will not provideAbortController either. Anybody who would want to use this feature will need to write one anyway.

On the other hand this polyfill (AbortController), will have no effect for browsers which supportfetch, because if nativefetch doesn't supportAbortController this polyfill will not help either.

Because of this inconsistency I end upusing your code as ponyfill.

I'm not convinced that it's required that we actually provide the AbortController polyfill. I know it's required for the abort feature to actually work, but adding those extra bytes for everyone for only a maybe use case kind of goes against the purpose of a tiny fetch polyfill/ponyfill. We can just say that support for abort requires an appropriate polyfill (like libs do with Promises) and point them elsewhere OR provide the above as a separate export in this module?

I have usedhttps://www.npmjs.com/package/abortcontroller-polyfill with this modified version of unfetch as a ponyfill to ensure consistent abort behaviour.

Another option would be to export AbortController separately:

importfetchfrom'unfetch';importAbortControllerfrom'unfetch/AbortController';

I've been thinking of doing the same forHeaders,Request andResponse.

@developit how are you thinking of doing that? I mean would you import and use the Request, Response and Headers implementations directly in the main unfetch function? Seems like that could be a lot of extra bytes... Or are you imagining them as completely separate entities?

This has been discussed in whatwg-fetch as well before, starting fromJakeChampion/fetch#592 (comment).

You might also want to look athttps://github.com/Netflix/yetch/blob/master/polyfill.js. It polyfills fetch wether it doesn't exist or wether it doesn't support abort controller.

My personal opinion as a user is what I mentioned inJakeChampion/fetch#592 (comment) - the fetch polyfill to implement entire fetch or add the missing parts (options.signal).

Lastly, why re-implementAbortSignal/AbortController (#94) when there are some fully WHATWG compliant polyfills out there already (e.g.https://github.com/mysticatea/abort-controller - which is whatnode-fetch will probably use -node-fetch/node-fetch#437).

Will this ever be merged, seems like the discussion got stuck.

Don't mean to be impetuous here, but - maybe we are overthinking this? We've been discussing it for almost two years. This is, literally, addingtwo lines of code to support something all major browser already do.

@nfantone AbortController is pretty well-supported now, yes. At the time of this original discussion it was only supported in Chrome. I think now we can merge this - unfetch is likely to get a little bump up in size in order to accomodate the addition ofHeaders,Response andRequest interfaces, which see increased usage nowadays.

options.signal.onabort = () => request.abort();

There might be a problem with this implementation. With native fetch, many requests can be cancelled with one abort call. If we overrideonabort, only one fetch will be canceled. Consider this example:

constctl=newAbortController();fetch('foo',{signal:ctl.signal}).then(action1).catch(()=>{});fetch('bar',{signal:ctl.signal}).then(action2).catch(()=>{});ctl.abort();

action1 may still be executed, as signal's 'abort' handler was overridden by the second call to fetch.
I'd recommend replacing.onabort = ... with.addEventListener('abort', ...). Alternatively, we could chain aborts:

constcurrent=options.signal.onabort||()=>{};options.signal.onabort=()=>{current();request.abort();}

I am not sure about side-effects of this one though.

Any update?
node-fetch added support toAbortController , now if we want to useisomorphic-unfetch with abortable request ,unfetch do not let us...
node-fetch/node-fetch#95

@developit@sayjeyhi I submitted PR#137 with alternative implementation. It solves problems I mentioned earlier at a cost of build size. If you decide to merge this (#68) PR without changes, please add a note to the README explaining what to expect from abort behaviour.

I think the two issues addressed in#137 make it preferable here - AbortController should be usable across multiple fetch() calls, and it should really reject with a value, since promise handlers are likely to be checking for such a value.

Will this ever be merged? :(