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

feat(useAxios): axios instance support, close #273 #342

Merged
merged 2 commits into from Feb 23, 2021
Merged

feat(useAxios): axios instance support, close #273 #342

merged 2 commits into from Feb 23, 2021

Conversation

wheatjs
Copy link
Member

@wheatjs wheatjs commented Feb 23, 2021

Adds support for Axios instances as mentioned in issue #273.

Uses the following overrides so that it works with the existing API without breaking it.

export function useAxios<T = any>(url: string, config?: AxiosRequestConfig): UseAxiosReturn<T>
export function useAxios<T = any>(url: string, instance?: AxiosInstance): UseAxiosReturn<T>
export function useAxios<T = any>(url: string, config: AxiosRequestConfig, instance: AxiosInstance): UseAxiosReturn<T>

let instance: AxiosInstance = axios

if (args.length > 0) {
if ('request' in args[0])
Copy link
Member

@antfu antfu Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a safe way to detect whether it's an axios instance?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to use instanceof but it looks like you can't do that(axios/axios#737) so I used the way that was suggested here. It doesn't seem to have any overlap with the AxiosRequestConfig, so it works from my testing. But I guess the only real way to enforce it is with the typescript types for parameters. If you have any ideas on another way to do it let me know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, go with it then. Can you add a comment above that line mentioning about the issue you linked? Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing!

@antfu antfu changed the title feat(useAxios): axios instance support feat(useAxios): axios instance support, close #273 Feb 23, 2021
@antfu antfu enabled auto-merge (squash) February 23, 2021 08:45
@antfu antfu merged commit 2689135 into vueuse:main Feb 23, 2021
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

Successfully merging this pull request may close these issues.

None yet

2 participants