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

Transform `async` functions to bluebird promises #4572

Merged
merged 1 commit into from Jul 13, 2017

Conversation

@richvdh
Copy link
Member

richvdh commented Jul 13, 2017

It turns out that the assertion made in #4565 about async functions returning bluebird promises was only correct when babel used an inline version of the asyncToGenerator helper; in riot-web we are using babel-transform-runtime which means that we use a separate babel-runtime/helpers/asyncToGenerator, which returns a native (or core-js) Promise.

This meant that we were still in the situation where some methods returned native Promises, and some bluebird ones, which is exactly the situation I wanted to resolve by switching to bluebird in the first place: in short, unless/until we get rid of all code which assumes Promises have a done method etc, we need to make sure that everything returns a bluebird promise.

(Aside: there was debate over whether in the long term we should be trying to wean ourselves off bluebird promises by assuming all promises are native. The conclusion was that the complexity hit involved in doing so outweighed any benefit of a potential future migration away from bluebird).

It turns out that the assertion made in
#4565 about `async` functions
returning bluebird promises was only correct when babel used an inline version
of the `asyncToGenerator` helper; in react-sdk we are using
`babel-transform-runtime` which means that we use a separate
`babel-runtime/helpers/asyncToGenerator`, which returns a native (or core-js)
Promise.

This meant that we were still in the situation where some methods returned
native Promises, and some bluebird ones, which is exactly the situation I
wanted to resolve by switching to bluebird in the first place: in short,
unless/until we get rid of all code which assumes Promises have a `done` method
etc, we need to make sure that everything returns a bluebird promise.

(Aside: there was debate over whether in the long term we should be trying to
wean ourselves off bluebird promises by assuming all promises are native. The
conclusion was that the complexity hit involved in doing so outweighed any
benefit of a potential future migration away from bluebird).
@lukebarnard1 lukebarnard1 merged commit 5e259ec into develop Jul 13, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@richvdh richvdh deleted the rav/async_to_bluebird branch Jul 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.