Simplify deferred/promise implementation

[vladar]

Merge Deferred and SyncPromise. This simplifies things a bit and allows certain new features (like Deferred chaining):

$resolver = function() {
    return Deferred::create(function() {
        return 'foo';
    })->then(function($foo) {
        return $foo . ' bar';
    });
}

The caveats are:

1. The sequence of nested deferred calls might be suboptimal and don't produce the desired batching sequence.
2. We can't wait for a specific promise anymore. The whole chain of promises has to be resolved before wait returns (as we have a single queue now).

So I encourage anyone using deferred resolvers to try this branch and report some feedback.

@mcg-web Can you check if this will affect your DataLoader implementation somehow?

CC @mfn @spawnia

Fixes #573

[simPod]

I like the simplification 👍

• Added few comments
• I think Deferred extends SyncPromise is a BC break as result of $deferred instanceof SyncPromise with this change changes from false to true so might need to document it.
• Do we need Deferred at all?

[mfn]

Wow, thanks for the quick actions. Unfortunately it will take me a while, seems due to dependencies I can't just upgrade to this commit and need to fiddle around manually.

Will keep you updated, also regarding the batch sequence order.

[spawnia]

Wow, thanks for the quick actions. Unfortunately it will take me a while, seems due to dependencies I can't just upgrade to this commit and need to fiddle around manually.

Will keep you updated, also regarding the batch sequence order.

Had a dependency issue as well and could resolve it by aliasing the reference:

"webonyx/graphql-php": "dev-simplify-deferred as 0.13.8"

[vladar]

I think Deferred extends SyncPromise is a BC break as result of $deferred instanceof SyncPromise with this change changes from false to true so might need to document it.

I agree that we should add a note about this in Upgrade.md

Do we need Deferred at all?

We do for now. First for BC, second to enforce executor function in the constructor (without it there is no place where we could start resolving a chain of promises).

[mcg-web]

Can you check if this will affect your DataLoader implementation somehow?

yes this will have an impact on DataLoader but a positive impact with this change it will make the implementation easier since we dealing with a unique object instead of two.

[spawnia]

@vladar i tried this branch in Lighthouse and ran our test suite against it, seems to be working fine.

[esamattis]

Hi! I been testing this with WPGraphQL. It's bit behind with graphql-php versions but this change in particular does not cause any issues with our tests.

I'm working on a field level caching solution for WPGraphQL at the moment and this change would help a lot with that since I need to tap into the promise chains so I can capture the resolved values.

Here's simplified version of what I'm trying to do

add_filter('graphql_resolve_field', function( $defer, $field_key ) {

	return $defer->then(function($result) use ($field_key) {
		write_cache( $field_key, wp_json_encode($result));
		return $result;
	});


}, 100, 2);

Where $defer is is GraphQL\Deferred

This does not work with the latest graphql-php stable release because the then method actually changes the type of $defer which seems break some instanceof check in downstream.

This PR fixes that issue nicely 👌