holen icon indicating copy to clipboard operation
holen copied to clipboard

Abort API

Open jamesplease opened this issue 7 years ago • 2 comments

Fetch will soon support aborting in evergreen browsers (and the popular GitHub polyfill will as well).

Given that this component can refetch after it is mounted, I don't think that accepting an abortController prop would work. This is likely to lead to errors, I think, because you would need to make sure that a new abort controller is made anytime the props change.

Instead, I think a better API would be something like:

<Holen createAbortController>
  ({ abortController }) => abortController.abort()
</Holen>

When createAbortController is true, then Holen will make a new abort controller, pass its signal into fetch, and then include the abortController in the render props.

Also, there could possibly be value in an abortOnUnmount prop.

What do you think? Thanks for reading!

jamesplease avatar Jan 20 '18 22:01 jamesplease

Given that this component can refetch after it is mounted, I don't think that accepting an abortController prop would work. This is likely to lead to errors, I think, because you would need to make sure that a new abort controller is made anytime the props change.

I'll need to think about it. I understand the why and you are on the right track, but something about it doesn't "click" as I look at it from an everyday use perspective.

The biggest problem is keeping track of which controller and signal belong to which fetch request. We need a way to link the 2 together somehow.

If we do this we should probably also add an onAbort prop that fires with the signal.

signal.addEventListener('abort', () => {
  // Logs true:
  // console.log(signal.aborted);
  // this.props.onAbort() <--
});

(from https://developers.google.com/web/updates/2017/09/abortable-fetch)


You are correct, abortOnUnmount should be a prop set to true by default.

tkh44 avatar Jan 22 '18 18:01 tkh44

The biggest problem is keeping track of which controller and signal belong to which fetch request. We need a way to link the 2 together somehow.

Good point. I hadn't considered this problem.

If we do this we should probably also add an onAbort prop that fires with the signal.

Something like this makes sense to me.

jamesplease avatar Jan 23 '18 03:01 jamesplease