Support for ES6 Promises #708
Conversation
…pre-4.x references
|
so what would it take to get this merged? |
|
as a review, I'd like to see the actual 4.x requirement done in a separate PR rather than hidden in this PR. |
ghost
mentioned this pull request
|
happy to apply any changes to merge this as required by current maintainers, which I guess are @sandeepmistry and @jacobrosenthal |
|
ok, I see there are a few conflicts. Happy to fix those if there is any indication that is the only thing blocking this from being merged. Otherwise, would love to know what else might be needed |
|
@MaxDesiatov : the Node 4.x changes are here #691, so when the time for rebase comes you'll need to remove your node 4.x changes. |
|
no problem, can do that as soon as #691 is merged |
|
@MaxDesiatov : It'll also need some examples when the go ahead is given. |
|
Touches a lot of code sadly (mostly because of whitespace) And sad we also have to touch our tests so much. Are people still doing callback AND promise apis? That would let all the existing tests pass to start with and give much more confidence. |
|
I think the nice part about doing both promises and callbacks is that folks can start using promises immediately and then we could deprecate the callbacks for 3.x |
ghost
mentioned this pull request
|
Please forward change to downstream fork: |
In this pull request all user-facing APIs that take callbacks now also return promises. This change is backwards compatible.
Some tests had to be changed, as previously there were expectations that callbacks were called immediately in the same event loop cycle. This is no longer true as callbacks are scheduled with promises, but shouldn't break any user code, which should have been written with an expectation that callbacks are asynchronous in the first place.
Additional tests were added to verify that all promises resolve successfully.
This fork is also published as https://www.npmjs.com/package/noble-promise in the meantime, as I see that main repository had no merging activity for some time. I still hope that this PR will be merged eventually.
@sandeepmistry could you please have a look at this PR? Thanks a lot for the library and I look forward to your feedback.