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

Correct use without deprecated GraphQL::execute()? #39

Closed
vhenzl opened this issue Nov 18, 2020 · 3 comments
Closed

Correct use without deprecated GraphQL::execute()? #39

vhenzl opened this issue Nov 18, 2020 · 3 comments

Comments

@vhenzl
Copy link
Contributor

vhenzl commented Nov 18, 2020

GraphQL::execute() has been deprecated many versions ago.

There are two examples of how to use dataloaders with webonyx/graphql-php (I'm aware of):

A test in this project:

$response = GraphQL::execute($schema, $query);

And https://github.com/mcg-web/sandbox-dataloader-graphql-php. But both use this deprecated API.

What's the correct use without deprecated GraphQL::execute() (particularly in combination with the "native" SyncPromise)?

GraphQL::executeQuery() can't be use as it internally always uses GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter\SyncPromiseAdapter instance, not Overblog\DataLoader\Promise\Adapter\Webonyx\GraphQL\SyncPromiseAdapter instance set with GraphQL::setPromiseAdapter() and it's execution results in "GraphQL\Error\InvariantViolation: Could not resolve promise" error.

Assuming from this comment, the whole use of GraphQL::setPromiseAdapter() should be considered deprecated and for all cases that don't use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter\SyncPromiseAdapter, GraphQL::promiseToExecute() should be used.

Given these adapters:

use Overblog\DataLoader\Promise\Adapter\Webonyx\GraphQL\SyncPromiseAdapter;
use Overblog\PromiseAdapter\Adapter\WebonyxGraphQLSyncPromiseAdapter;

$graphQLPromiseAdapter = new SyncPromiseAdapter();
$dataLoaderPromiseAdapter = new WebonyxGraphQLSyncPromiseAdapter($graphQLPromiseAdapter);

I see two possible solutions. Either:

$promise = GraphQL::promiseToExecute($graphQLPromiseAdapter, $schema, $query /* ... */);
$result = $graphQLPromiseAdapter->wait($promise);

which kinda replicates what GraphQL::execute() does, or:

$promise = GraphQL::promiseToExecute($graphQLPromiseAdapter, $schema, $query /* ... */);
$result = DataLoader::await($promise)->toArray();    

Is that correct? Both works and seems to work equally. Is there any real difference between them? Any pros and cons using one over the other?

@vhenzl
Copy link
Contributor Author

vhenzl commented Nov 18, 2020

#36 likely related.

@vhenzl
Copy link
Contributor Author

vhenzl commented Nov 19, 2020

The second option with DataLoader::await() doesn't seem to be correct. It can fail with "Found no active DataLoader instance" error if no field returning a promise queried - for instance during introspection.

mcg-web pushed a commit that referenced this issue Dec 7, 2020
* Drop hhvm from build matrix

* Bump minimal PHP version to 7.1

* Remove PHP 7.3 from allowed failures

* Add PHP 7.4 to build matrix

* Check platform requirements

* Replace `\PHPUnit_Framework_TestCase` with `\PHPUnit\Framework\TestCase`

* Bump PHPUnit to version 7.5

* Bump `webonyx/graphql-php` to version 0.13

* Prefer package sources on `composer update`

`tests/StarWarsData.php` is no longer part of `webonyx/graphql-php` package

* Fix deprecation for `GraphQL\Schema`

* Suppress deprecation errors in tests

* Bump `webonyx/graphql-php` to version 14

* Bump PHPUnit to version 8.5

* Allow PHPUnit 7 for compatibility with PHP 7.1

* Allow PHP 8

* Use current version of `php-cs-fixer`

* Switch Travis CI from `trusty` to `bionic`

* Add PHP 8.0 to the build matrix

* Don't run `php-cs-fixer` for PHP 8.0

Resolves #40.

Minimal PHP version increased to 7.1. (So that supported versions are same as in webonyx/graphql-php. Going up to 7.2 would make things easier regarding PHPUnit.)
Installation on PHP 8 allowed.
composer check-platform-reqs added to build steps
PHPUnit bumped to ^7.5|^8.5 to support PHP 7.1-8.0
webonyx/graphql-php updated to the current version 14
--prefer-source used for composer update as tests/StarWarsData.php is no longer part of webonyx/graphql-php package. Alternative options would be to create a local copy of the file in this repository or download it from webonyx/graphql-php repo with wget in before_install.
The current version of php-cs-fixer (2.16) used, however, it doesn't allow installation on PHP 8 yet (see PHP-CS-Fixer/PHP-CS-Fixer#4702).
PHPUnit option convertDeprecationsToExceptions disabled to deal with deprecated GraphQL::execute() (related to #39).
@simPod
Copy link
Contributor

simPod commented Mar 13, 2021

I have dropped deprecated api in tests in #51

@mcg-web mcg-web closed this as completed Nov 22, 2021
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

3 participants