Adds support for asynchronous executions via Promises

[acelot]

• New promiseExecute method in Execution.php
• Helpers for API (graphqlAsync(), processAsync)

[Jalle19]

Would it be possible to use the async version all the time, and just change the graphql helper function to wait for the result? That way there would be a lot less code duplication.

[acelot]

@Jalle19 I think so too, but did so for backward compatibility

[Jalle19]

@Jalle19 I think so too, but did so for backward compatibility

You can think of the graphql() helper function and it's related functions as the public API, not the actual public interfaces etc.

[acelot]

It seems to me that synchronous and asynchronous methods should exist separately, since impossible to turn asynchronous promise into synchronous, because the result of promise comes through a callback.

[Jalle19]

Isn't it possible to just add ->then() and return the result to turn it into synchronous?

[acelot]

Nope. In any case, will return Promise.

return GraphQL::promiseExecute(...)->then(function (ExecutionResult $result) {
    return $result;
})

$result = graphql(...);
var_export($result instanceof Promise); // true

[Jalle19]

Isn't there some wait method that could be used?

[acelot]

All promises works in synchronous way without event loop. But thanks to the ReactPHP event loop PHP works in real asynchronous mode. Unfortunately I don't know how ReactPHP resolves promises.

[acelot]

I have refactored the code. Now execute method in Execution always returns a Promise. I have moved promise resolving for synchronous calls into API method graphql. I also added a check, if a Event Loop is used (React PHP), an error is returned.

[acelot]

Any chance that this PR will be merged?

[Jalle19]

I'll try to find some time to test this out this week, the latest changes look very promising (no pun intended)!

[Jalle19]

@acelot I fixed the tests, for some reason Travis CI doesn't run on this pull request. What's the next step in getting deferred resolving to work?

[acelot]

https://github.com/lordthorzonus/php-dataloader works for me very well. But my project based on ReactPHP. I have not tested data loader in regular PHP environment.

[Jalle19]

I still need to test this in a non-asynchronous environment before I dare merge this, I'm not super familiar with part of the code. I have to think if we really want to break the ExecutionInterface signature even though I said it was fine earlier.

[crisu83]

Hello @acelot,

First of all, I'd like to thank you for all the work you've put into this. I'm going through a divorce so I haven't had any energy to focus on this project for quite some time now, but now I'm back at work after a long vacation so I started working on a version 1.2 yesterday.

I agree that separate methods are the way to go, and we definitely want to hand over the promise from the public API, instead of resolving it before returning the result. This gives the consumer the freedom to do whatever they need with that promise.

I will now go through the changes and add my comments. After that, if all is good and no more changes are required we can merge this. Thanks once again for your efforts! 😊

[crisu83]

We had some problems with deferred resolvers (#336). Any change you've had any success with them @acelot? The problem was that the promises got resolved too early, instead of using a dictionary, delaying the resolution and resolving them all at once. Did you find a way to fix the deferred resolvers? Thanks in advance.

[crisu83]

I added some comments, but they mostly nit-picking. Change them if you feel like it, I can also fix them afterwards if you're not up for it. 🙂