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

Add some features from useFetch also to useAxios #449

Closed
sethidden opened this issue Apr 13, 2021 · 8 comments
Closed

Add some features from useFetch also to useAxios #449

sethidden opened this issue Apr 13, 2021 · 8 comments

Comments

@sethidden
Copy link

sethidden commented Apr 13, 2021

Currently useFetch has execute(), but useAxios does not. This means that with useFetch you can first create useFetch, set {immediate: false}, then execute() the call later, but with useAxios it fires immediately when you call useAxios.

Abort is also missing in useAxios.

Some properties don't make sense to add into axios such as statusText (since that's just in the axios response).

Another thing is that the returned property names are different in useAxios and useFetch, but changing that'd be a breaking change.

Exposed logic from both logics:

useAxios
  response: Ref<AxiosResponse<T> | undefined>
  data: Ref<T | undefined>
  finished: Ref<boolean>
  loading: Ref<boolean>
  canceled: Ref<boolean>
  error: Ref<AxiosError<T> | undefined>
useFetch
  isFinished: Ref<boolean>
  statusCode: Ref<number | null>
  response: Ref<Response | null>
  error: Ref<any>
  data: Ref<T | null>
  isFetching: Ref<boolean>
  canAbort: ComputedRef<boolean>
  aborted: Ref<boolean>
  abort: Fn
  execute: () => Promise<any>
@sethidden
Copy link
Author

Or perhaps it's better to just use https://vueuse.org/core/useAsyncState/ for requests in general?

@lstoeferle
Copy link
Member

Hey @3nuc,

I would love to see the execute() logic in useAxios too. AFAIK there is already an option to cancel request in useAxios (I can remember me adding it).

But I don't see any value in introducing breaking changes while refactoring the return type to match useFetch. IMO there are only a few rare cases, where somebody is using axios besides the fetch api and vice versa. Typically you only use one of both, which eliminates the risk of mixing the return types of useAxios and useFetch.

Feel free to open a PR 😉

Kind regards,
Lukas

@antfu
Copy link
Member

antfu commented Apr 13, 2021

Would love to align those interfaces. We can make aliases and for APIs and mark some as deprecated. Then drop them in along with the next major.

@wheatjs
Copy link
Member

wheatjs commented Apr 13, 2021

Which one are we looking to align against? Make useAxios align with the useFetch interface or useFetch align with the useAxios interface?

@antfu
Copy link
Member

antfu commented Apr 14, 2021

Aligning with useFetch looks better to me

@wheatjs
Copy link
Member

wheatjs commented Apr 15, 2021

Sounds good, I'll make a PR sometime this week!

@raphaelsaunier
Copy link

Thanks a lot for starting to work on this @wheatjs!

Supporting { immediate: false } and execute() would make a lot of sense in situations where you want to trigger the request conditionally while still having access to the various properties exposed by useAxios in your setup method.

In your PR description you mention that you'd like to avoid adding more overrides or other config options. As far as I can tell, this could simply be done in an additive way with the existing and optional immediate property, right? I'm not very familiar with the project so maybe I'm missing an obvious limitation.

@w20k
Copy link

w20k commented Sep 8, 2021

Thanks a lot for a great library!

Supporting beforeFetch, afterFetch and a new one that I've added - afterFetchFail would be a great addition to useAxios. For now in my dev project those are hacks that I've added to an existing method - useAxios.

My main goal is to separate store logic and UI, and build a simple optimistic update flow with an option to update (mutate) store in a callback function, even if the main caller was closed (in my case a parent popup).

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

6 participants