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

[useAsyncFn] Returning an error is "handling" the error #1768

Open
avioli opened this issue Jan 25, 2021 · 6 comments
Open

[useAsyncFn] Returning an error is "handling" the error #1768

avioli opened this issue Jan 25, 2021 · 6 comments

Comments

@avioli
Copy link

avioli commented Jan 25, 2021

I'm not sure the return line is doing the correct thing, but that might be the intended behaviour.

(error) => {
isMounted() && callId === lastCallId.current && set({ error, loading: false });
return error;
}

Returning anything other than a Promise.reject(...) from a catch() results in "handling" the error, thus resolving the promise successfully... with the returned value.

That line should either throw error or return a Promise.reject(error).

Given that there are no issues about it so far, I'm guessing current state is a ok. Feel free to close if so.

@Mookiepiece
Copy link

same problem, but I found #612

@avioli
Copy link
Author

avioli commented Feb 2, 2021

I don't know how I missed that issue?

I think the right thing to do is to not return anything or return a new Promise, which will resolve with no result after the original is fulfilled, be it rejecting or resolving. This way the point of the useAsync promise is not to give you a result (error or not), but to let your code await for it to finish.

The whole point of useAsync is to give you the new state of the promise on each re-render, so you can react to those using hooks. So in essence you should not be awaiting on it.

@Mookiepiece
Copy link

That would be type friendly.

I thought useAsyncFn would decotate my fn, but It returns a trigger instead of a superFn. I just accept that and create my own.

@avioli
Copy link
Author

avioli commented Feb 3, 2021

Nah... I see the main point of hooks as state holders + mutators (most often).

useAsyncFn is no different - a state holder for the promise' state and result (be it an error or not) and a trigger, which initiates the whole process of mutation of said state.

The beauty of hooks is that they (mostly) are short and one can read their source code and see how they work.

@vikitaoshunyi
Copy link

vikitaoshunyi commented Jul 22, 2021

you can return Promise.reject(Promise.reject('error')); looks like this

const [{ value: tableData }, fetchFunc] = useAsyncFn(async query => {
  const [err, res] = await getData(query);
  if (err) {
    return Promise.reject(Promise.reject('error'));
  }
  return res;
});
...
const onClick = () => {
  fetchFunc()
    .then(res => {
      console.log(res);
    })
    .catch(e => {
      console.log(e);
    });
}

@JoeDuncko
Copy link

Hi all! @react-hookz/web, the new library by one of react-use's former maintainers (background here and here) has implemented a useAsync hook that is much more ergonomic and typescript friendly. You might want to check it out!

For those interested, there's an official migration guide for migrating from react-use to @react-hookz/web.

Hope this helps!

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

No branches or pull requests

4 participants