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

Docs are a bit confusing re: load / all #22

Closed
scott-r-lindsey opened this issue Apr 9, 2019 · 5 comments
Closed

Docs are a bit confusing re: load / all #22

scott-r-lindsey opened this issue Apr 9, 2019 · 5 comments

Comments

@scott-r-lindsey
Copy link

Hi there, I'm trying to get up to speed on GraphQL and I think either the example in the README is wrong, or else maybe could just use some clarification for n00bs such as myself.

Specifically, the "Usage in a resolver" seems to be calling the "load" function of ShipLoader, but of course ShipLoader does not have a "load" function, only an "all" function.

Is there supposed to be an intermediate object, or some functions added via inheritance?

@scott-r-lindsey
Copy link
Author

And I see that, per #20 I am not the only one. This project seems to have some issues.

@mcg-web
Copy link
Member

mcg-web commented Apr 9, 2019

@scott-r-lindsey you should have a look to the base library ?this bundle is just a way of implementing this in symfony.

@scott-r-lindsey
Copy link
Author

I have, thank you but I was really hoping to be able to use this bundle rather than have to create my own Symfony integration.

Perhaps it is the intent that the ShipLoader in the example is meant to extend Overblog\DataLoader\DataLoader? That seems closer, but the constructor doesn't seem to be right and it doesn't seem to be getting the right arguments from the bundle.

@mcg-web
Copy link
Member

mcg-web commented Apr 9, 2019

here a full example

batch_load_fn is just a callable service returning a fulfilled promise using the default promiseAdapter

<?php
namespace App\Loader;

use App\Repository\BlogRepository;
use GraphQL\Executor\Promise\PromiseAdapter;

class BlogLoader
{
    private $promiseAdapter;

    private $blogRepository;

    public function __construct(PromiseAdapter $promiseAdapter, BlogRepository $blogRepository)
    {
        $this->promiseAdapter = $promiseAdapter;
        $this->blogRepository = $blogRepository;
    }

    public function __invoke(array $blogIDs)
    {
        $blogList = $this->blogRepository->getBlogsByIds($blogIDs);
         // indexing by blog id
        $blogListByIDs = [];
        foreach ($blogList as $blog) {
            $blogListByIDs[$blog->getId()] = $blog;
        }
        // the same order than $blogIDs must be follow here.
        $results = [];
        foreach ($blogIDs as $id) {
            $results[] = $blogListByIDs[$id] ?? null;
        }

        return $this->promiseAdapter->all($results);
    }
}

Config:

overblog_dataloader:
    defaults:
        promise_adapter: 'overblog_dataloader.webonyx_graphql_sync_promise_adapter'
        options:
            max_batch_size: 50
    loaders:
        blogs:
            alias: 'blogs_loader'
            batch_load_fn: '@App\Loader\BlogLoader'

Usage

Query:
    type: object
    config:
        fields:
            blog: {type: "Blog!", resolve: "@=service('blogs_loader').load(rootValue.id)"}

@scott-r-lindsey
Copy link
Author

Oh this was helpful. Thank you. I finally got my n+1 issues figured out.

I think my real problem was that I was trying (and failing) to translate the docs for overblog/GraphQLBundle from SF 2/3 to SF 4, with modern autowiring.

For me, the light went on when I saw your "resolve: "@=service('blogs_loader')..." which gave me the hint that there could be a better way to instantiate my resolvers.

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

2 participants