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

Ponyfill Promise with pinkie-promise #250

Closed
wants to merge 4 commits into from
Closed

Conversation

floatdrop
Copy link

Alternative way to support Node.js 0.10 (see #244).

We are using co in ava test framework and polyfilling global Promise object is not working for us, because it will affect code under testing. If code under testing relies on Promise object and we polyfill it globally it will pass tests on Node.js 0.10, but will fail in production.

This approach supports polyfilling global Promise with global.Promise = require('bluebird'); and returns native promises (if available). If neither works it will return small pinkie polyfill.

@sindresorhus
Copy link

👍

@fengmk2
Copy link
Collaborator

fengmk2 commented Nov 1, 2015

No, let's forget 0.x of node.
It's time to use node 4.x.

@sindresorhus
Copy link

Personally yes, but unrealistic, at least for us, seeing as Node.js 0.10 is LTS until October 2016.

@fengmk2
Copy link
Collaborator

fengmk2 commented Nov 2, 2015

If you want to use node 0.10.x, you need to set Promise to global, it's not co need to make sure Promise exists.

global.Promise = global.Promise || require('pinkie');
var co = require('co');

https://github.com/tj/co/blob/master/package.json#L33 see engines too.

@floatdrop
Copy link
Author

@fengmk2 this is not working for us, because global.Promise = ... will pollute global scope and code, that uses Promise object in Node.js 0.10 will pass tests but will fail in production.

@madbence
Copy link
Contributor

madbence commented Nov 2, 2015

the readme describes well that you need a Promise implementation, imho this is outside of cos scope

@floatdrop floatdrop closed this Nov 15, 2015
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.

4 participants