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

Make axios instantiable #123

Merged
merged 3 commits into from
Oct 27, 2015
Merged

Make axios instantiable #123

merged 3 commits into from
Oct 27, 2015

Conversation

nickuraltsev
Copy link
Member

Fixes #122, #108

@angelxmoreno
Copy link

👍

@mzabriskie
Copy link
Member

Sorry for the delayed response. I totally thought I had already replied.

I really like this PR! This has been something that I've been meaning to do for a while.

I would suggest a couple minor changes.

  • I would prefer axios.create over axios.createNew for the factory method.
  • What is the purpose for axios.request? Looks like it takes the place of calling axios directly. I would prefer not to loose this functionality since axios "competes" with fetch, and I would like to preserve a similar, succinct API.

@nickuraltsev
Copy link
Member Author

@mzabriskie - thank you for looking at this!

I will rename axios.createNew to axios.create.

As for request, it doesn't replace the existing API. All the existing static methods including axios(), axios.get(), axios.post() are retained. The difference between request() and axios() is that the former is an instance method. It provides the same functionality, but is called on an instance and uses the default configuration of that instance. This method can be useful in case when you would like to use the new instance API rather than the static one, but the shortcut methods like get and post don't work for you for some reason (e.g., you need to use a non-standard HTTP method). I'm ok with removing it though. What do you think?

@mzabriskie
Copy link
Member

Thanks for this PR!

mzabriskie added a commit that referenced this pull request Oct 27, 2015
@mzabriskie mzabriskie merged commit 68b1b37 into axios:master Oct 27, 2015
@nickuraltsev
Copy link
Member Author

You are welcome!

@aripalo
Copy link

aripalo commented Nov 1, 2015

Any change you could publish new version of Axios into npmjs.org with this instantiation feature? I am actually in dire need of this :) (Okay, I'll survive, I can use axios as git depedency for now)

@axios axios locked and limited conversation to collaborators May 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make axios instantiable
4 participants