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

AbortController review #13

Closed
franciscop-sc opened this issue Oct 29, 2019 · 3 comments
Closed

AbortController review #13

franciscop-sc opened this issue Oct 29, 2019 · 3 comments
Labels
enhancement New feature or request

Comments

@franciscop-sc
Copy link
Contributor

franciscop-sc commented Oct 29, 2019

Hi, it's me again :)

I just wanted to stop by to say I've been trying a new API, and I think use-async-effect could consider supporting the AbortController API. It seemed an opaque API for me before, but after experimenting a bit it seems like a very similar example to the useRef() API:

var controller = new AbortController();
var signal = controller.signal;
console.log(signal);
// { aborted: false, onabort: null }

document.querySelector('body').addEventListener('click', e => {
  console.log(signal);
  // { aborted: false, onabort: null }
  controller.abort();
  console.log(signal);
  // { aborted: true, onabort: null }
});

I am not sure how (if any) to use it, but I can see maybe providing it as a second parameter:

useAsyncEffect(async (isMounted, signal) => {
  try { 
    const res = await fetch(`/users/${id}`, { signal });
    const data = await res.json();
    if (!isMounted()) return;
    setUser(data);
  } catch (error) {
    console.error(error);
  }
}, []);

On the other hand, I think these are bad:

  • Mixing try/catch with normal if checks for handling the lifecycle. It seems to be the way fetch() works, but it's very ugly.
  • Signal has an onabort callback, which is very similar to the second callback in use-async-effect. Again, two different ways of doing almost the same.
  • It can probably be done in user-land in a not-difficult way (going to attempt this soon). The main advantage of doing it this way is the availability of passing it straight into fetch(), which seems the most common case for use-async-effect.
@houfio
Copy link
Collaborator

houfio commented Jan 30, 2020

Hey, sorry for the very late reply! This sounds like a great idea, but I'm not sure if we should implement it into the library itself. Have you already done some experimentation with your own implementation?

@houfio houfio added the enhancement New feature or request label Jan 30, 2020
@dartess
Copy link

dartess commented May 13, 2020

AbortController is proposed to be used in another implementation useAsyncEffect: https://github.com/n1ru4l/use-async-effect.

This implementation looks more concise. There is no need to use experimental API and generators.

@franciscop-sc
Copy link
Contributor Author

BTW I ended up implementing this as part of a growing module I've published:

import { useAsyncEffect } from "use-async";

useAsyncEffect(async signal => {
  const res = await axios.get('/data', { signal });
  setProfile(res.data);
}, [id]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants