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

Allow using native promises? #27

Open
MichalCz opened this issue Mar 26, 2019 · 7 comments
Open

Allow using native promises? #27

MichalCz opened this issue Mar 26, 2019 · 7 comments

Comments

@MichalCz
Copy link

Hi,

First of all: I really like the way you wrote this bridge - I was on my way of writing the same thing on my own so this is seriously cool to see. :)

I see that your code depends on bluebird - which is quite heavy when compared to native promises. It has the upshot of being available anywhere, but perhaps you'd consider moving to native promises with some polyfill like any-promise and add bluebird as optional dependency?

I would be happy to help.

@JYone3A
Copy link

JYone3A commented Apr 9, 2019

+1

@munro
Copy link
Member

munro commented Apr 10, 2019

I would be happy to help.

@mickdekkers yessss that would be awesome!

@huan
Copy link
Collaborator

huan commented Apr 10, 2019

Thanks for the suggestion!

A pull request that could demo how to improve this issue will be welcome. Please feel free to submit solutions so that we could be able to start a discussion based on that.

@MichalCz
Copy link
Author

Hey. I tried a little but I'd need to dig in a bit more to make this work - the issue is in the use of two Bluebird methods try and finally (the latter is available in node v10). I don't want to add any polyfills as a dependency so I'll need to drill deeper into the issue...

...and I don't know when I'd find some time...

munro pushed a commit that referenced this issue Apr 24, 2019
@munro
Copy link
Member

munro commented Apr 24, 2019

@MichalCz I was able to remove bluebird in #28—which yea removes removes/changes some functionally (especially the .catch(...) semantics). I'm ok with that since I personally use async/await, and then the default behavior will be the built in Promise, which makes sense, because then people can wrap it with bluebird or whatever if they choose to.

The remaining work is to

  • update the readme to reflect the changes (some of the examples will no longer work)
    • and since they have to be updated I think it make sense to show async/await now!
  • make sure all the readme examples are defined as tests
  • make sure all the tests pass!

I'm about to sign off for tonight, but any help would be greatly appreciated :D

@MichalCz
Copy link
Author

@munro I'll check that out. :)

@munro
Copy link
Member

munro commented Feb 5, 2021

image

Just got some expert insight on how to better handle this transition 🎉

@munro munro mentioned this issue Feb 5, 2021
5 tasks
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

No branches or pull requests

4 participants