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

Configure the promise implementation #44

Closed
torarnek opened this issue Feb 24, 2015 · 32 comments
Closed

Configure the promise implementation #44

torarnek opened this issue Feb 24, 2015 · 32 comments
Milestone

Comments

@torarnek
Copy link
Contributor

I wish to provide my own Promise implementation (core-js), so I do not want browserify do bundle the es6-promise module. Is there a simple way to configure axios not to require this module?

// Polyfill ES6 Promise if needed
require('es6-promise').polyfill();
@torarnek torarnek changed the title Avoiding including Configure the promise implementation Feb 24, 2015
@torarnek
Copy link
Contributor Author

One possible approach could be something similar to:
https://github.com/spoike/refluxjs#switching-promise-library

@hoppula
Copy link

hoppula commented Feb 26, 2015

+1 for possibility to switch promise implementation, using native-promise-only in many projects, this would reduce the bundle size.

@pstoica
Copy link

pstoica commented Mar 2, 2015

In webpack or browserify, alias the package to:

axios/dist/axios.standalone.js

It will be prebuilt, but it won't require a separate promise. Promise must be defined globally.

@pstoica
Copy link

pstoica commented Mar 2, 2015

Oops, just tried it and the standalone build still assumes Promise.polyfill exists.

@mzabriskie
Copy link
Member

@torarnek why do you want to use your own Promise lib? The idea behind using es6-promise is that since it's just a poyfill, I can drop the dependency in the future once all browsers support it natively. In the mean time any browser that already supports native Promises will use the native implementation.

@mzabriskie
Copy link
Member

@hoppula axios is already using native Promise if it exists. If you're just trying to reduce the bytes, you can use dist/axios.standalone.js which doesn't bundle Promise.

@pstoica
Copy link

pstoica commented Mar 2, 2015

I'm using babel and its runtime. Calls to Promise are swapped with babel-core's Promise. So I already have something taking care of polyfilling.

@mzabriskie
Copy link
Member

@pstoica sounds like a bug. Shouldn't be calling Promise.polyfill when standalone.

@pstoica
Copy link

pstoica commented Mar 2, 2015

Sounds good. How about making a __STANDALONE__ flag with DefinePlugin to remove that?

@kentor
Copy link

kentor commented Mar 3, 2015

Would also be interested in this. I'm using browserify and would like to use bluebird as the promise implementation for axios since it supports finally.

@mzabriskie
Copy link
Member

@pstoica standalone build is fixed with 0.5.1

@pstoica
Copy link

pstoica commented Mar 12, 2015

Great, thanks so much! Seems like this can be closed now?

@kentor
Copy link

kentor commented Mar 13, 2015

I'm using browserify. require('axios/dist/axios.standalone') just returns an empty object?

@mzabriskie
Copy link
Member

@kentor the files under dist/ are meant for either global <script src="axios/dist/axios.js"></script> or for use with AMD require(['axios'], function (axios) {});.

For CJS there currently isn't an option, which is where I think this issue is stemming from.

If you want bluebird just to use finally I would recommend looking at this recipe.

@pstoica
Copy link

pstoica commented Mar 13, 2015

It sounds like browserify is picking up the file fine. Wouldn't one UMD build fix all these issues? (I didn't realize axios.js was one var declaration.)

@hoppula
Copy link

hoppula commented Mar 13, 2015

Thanks @mzabriskie, standalone build works great with browserify after the fix.
@kentor you should just require('axios') and then in your package.json:

really late edit: @kentor you're right, my build hadn't been properly updated, can confirm that it just returns an empty object.

@kentor
Copy link

kentor commented Mar 13, 2015

@hoppula that work for you? I tried it and it's the same, I just get a {}. I think specifying an alias in the browser field is basically the same as explicitly calling require('axios/dist/axios.standalone.js').

@mzabriskie thanks that recipe does work indeed. Though, I guess I will need to use the same promise implementation as the one in axios in other places in my code if I want to save bytes in my bundle.

@pstoica
Copy link

pstoica commented Mar 13, 2015

You're getting an empty object because nothing is being exported yet. One UMD build will give you either AMD, CommonJS, or a global. It also means you wouldn't have to specify AMD/etc in the filename.

@mzabriskie
Copy link
Member

I have reworked the bundling to be UMD compatible. While this will allow requiring dist/axios.standalone.js using CJS I have a couple reservations about moving forward with this.

  1. UMD falls apart under specific conditions. Let's assume we're working in a legacy system, where everything is dumped onto global. In an effort to clean things up we introduce requirejs and start pulling in dependencies with AMD. In any sizable app this cannot be done overnight, so we have a migration strategy, and start converting things over time. As soon as requirejs is on the page any UMD module is immediately unavailable globally. So hypothetically if we wanted to keep axios global until all our project was migrated to AMD, then using UMD for axios just broke the app. The reason for this is that UMD does a test like if(typeof define === 'function' && define.amd). Even if we are loading our UMD module with a script tag, it's going to use the AMD loading strategy.
  2. In order to save bytes in the browser, everything under dist/ ignores the node code path. It is assumed that if you are using node you will just be doing var axios = require('axios'); and there will be no need for a bundled module as is needed in the browser. This means that even though UMD will support use with CJS, doing var axios = require('axios/dist/axios.standalone'); will try and use the XHR adapter, not HTTP as expected. So you're still stuck using the polyfilled Promise.

I would prefer using UMD as it simplifies dist/, and greatly simplifies the build. At the same time I don't want to do it at the cost of introducing new headaches.

@mzabriskie mzabriskie mentioned this issue Mar 19, 2015
@mzabriskie mzabriskie added this to the 0.6.0 milestone Mar 19, 2015
@nelix
Copy link

nelix commented Mar 19, 2015

Can you not just not require es6-promise (by default instead of in the global build) and add something to the readme telling the consumer to use babel or es6-promise?

@mzabriskie
Copy link
Member

@nelix that would be preferable in a lot of ways. We could switch es6-promise to be a peerDependency, then it's readily available if needed.

Anyone else have any opinions on this? It would not be backward compatible, which I'm okay with. This could be communicated with an upgrade guide. The biggest factor is just pushing off the Promise dependency management to the developer consuming axios. At the same time, it allows more flexibility.

@pstoica
Copy link

pstoica commented Mar 20, 2015

Yeah, I think at this point it'd be better to ask people to responsibly manage their own Promise/ES6 polyfills. I think people would be using Promises in general if they decided to use axios.

@kentor
Copy link

kentor commented Apr 17, 2015

I'm 👍 on removing the promise from the library. I would even go as far as not specifying it as a peerDependency. I'm with facebook's on this one:

https://facebook.github.io/react/docs/working-with-the-browser.html#browser-support-and-polyfills

we've also taken the stance that we, as authors of a JS library, should not be shipping polyfills as a part of our library. If every library did this, there's a good chance you'd be sending down the same polyfill multiple times, which could be a sizable chunk of dead code.

@tomatau
Copy link

tomatau commented Apr 21, 2015

+1 to remove polyfills from library.

@AndrewRayCode
Copy link

Please make axios use standard finally :)

@NervosaX
Copy link

NervosaX commented Jun 9, 2015

+1 for allowing me to change the promise implementation.

@ascrazy
Copy link

ascrazy commented Jun 15, 2015

👍 on removing es6-promise from the library.

@mzabriskie
Copy link
Member

Once 0.6.0 has been released you will need to provide your own promise implementation.

@CrisLi
Copy link

CrisLi commented Sep 25, 2015

0.6.0 has released, and in the upgrade guide, only es6-promise has been mentioned.

require('es6-promise').polyfill();
var axios = require('axios');

How to config axios using bluebird?

@bboydflo
Copy link

@CrisLi have you found out how to configure axios to rely onBluebird as promise library?

@CrisLi
Copy link

CrisLi commented Mar 17, 2017

@bboydflo

Sorry, I have switched to use Fetch API.

@bboydflo
Copy link

@CrisLi I solved the issue by checking if Promise attribute is in the global scope, otherwise, window.Promise =BluebirdPromise;

@axios axios locked and limited conversation to collaborators May 21, 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

No branches or pull requests