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

Conversation

@nickuraltsev
Copy link
Member

commented Oct 3, 2015

Fixes #122, #108

@angelxmoreno

This comment has been minimized.

Copy link

commented Oct 14, 2015

👍

@mzabriskie

This comment has been minimized.

Copy link
Member

commented Oct 22, 2015

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

This comment has been minimized.

Copy link
Member Author

commented Oct 22, 2015

@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

This comment has been minimized.

Copy link
Member

commented Oct 27, 2015

Thanks for this PR!

mzabriskie added a commit that referenced this pull request Oct 27, 2015

Merge pull request #123 from nickuraltsev/master
Make axios instantiable

@mzabriskie mzabriskie merged commit 68b1b37 into axios:master Oct 27, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nickuraltsev

This comment has been minimized.

Copy link
Member Author

commented Oct 27, 2015

You are welcome!

@aripalo

This comment has been minimized.

Copy link

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.