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

Use native Promises #294

Closed
wants to merge 1 commit into from
Closed

Conversation

nickserv
Copy link

@nickserv nickserv commented Sep 9, 2017

Fixes #293.

  • Replaces Bluebird (a relatively large dependency) with native Promises, resulting in better standard support and likely performance.
  • Uses a shim for Node 8's require('util').promisify which supports custom Promises. This shim can be replaced with the native function after all Node versions before 8 are unmaintained (April 2019).
  • Simplifies our promisify function (which promisifies if a callback is not given) by using util.promisify internally.
  • Our promisify now returns nothing (instead of a malformed Promise which does not reject or resolve) when given a callback.
  • Stops testing Node 0.10, which is no longer in LTS anyway, because it doesn't support Promises.

@nickserv nickserv changed the title Replace Bluebird with native Promises and our promisify function WIP: Replace Bluebird with native Promises and our promisify function Sep 10, 2017
@nickserv nickserv changed the title WIP: Replace Bluebird with native Promises and our promisify function WIP: Replace Bluebird with native Promises Sep 10, 2017
@nickserv nickserv changed the title WIP: Replace Bluebird with native Promises Replace Bluebird with native Promises Sep 10, 2017
@nickserv nickserv changed the title Replace Bluebird with native Promises Use native Promises Sep 11, 2017
@nickserv
Copy link
Author

I'm done with this, and all the tests are passing. I would appreciate a review/merge soon so can migrate Koala and other projects using this away from Bluebird Promises completely.

@nickserv nickserv force-pushed the use-native-promises branch 2 times, most recently from 9b2574e to 4c1f611 Compare September 11, 2017 09:13
@doowb
Copy link
Collaborator

doowb commented Sep 18, 2017

@nickmccurdy thanks for the PR. I'm reviewing it as I'm working on other PRs, but I have a couple of questions:

  • why do you need to remove bluebird?
  • if bluebird isn't used, is there another way to support older node versions?

@niftylettuce
Copy link
Collaborator

If you fix conflicts, please submit a new PR

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.

Use native Promises
3 participants