folkloreinc / laravel-graphql

Facebook GraphQL for Laravel 5. It supports Relay, eloquent models, validation and GraphiQL.
1.76k stars 234 forks source link

Relay Modern support #135

Open Gugudesaster opened 7 years ago

Gugudesaster commented 7 years ago

Hi,

thanks for your awesome package and the hard work. Relay modern is out with a few small changes. Are there any plans to support that in the near future?

They also finalized a draft for subscription support? Are there any ideas on how to implement that? Do you think we could get it working with the laravel broadcasting feature and maybe a custom node server?

https://facebook.github.io/relay/docs/new-in-relay-modern.html

dmongeau commented 7 years ago

The work has been started in feature/relay branch. I haven't follow the release of Relay 1.0 but from what I've seen, it should mostly be compatible. The feature/relay branch is "ready" but missing 100% code coverage with tests. I would try to release it in the coming month, but in the mean time, any pull request on this branch is welcome!

SonarSoftware commented 7 years ago

Implementing subscriptions would definitely be a great feature!

4levels commented 6 years ago

Hi @dmongeau,

I'm currently trying to get your project play nice with Relay Modern. Everything seems to work (using the relay branch) but I have an issue with the Node and Viewer fields as described here: https://www.learnrelay.org/queries/what-is-a-query/ Almost all tutorials I find online are using this design (left alone if it's usefull for public data) and since your project already handles the Node field perfectly, I was wondering how I should go about implementing the Viewer field. Should I add it manually to my Queries / Types or do you intend to automate this in a similar way as the Node field?

As I'm no expert with React, I'd love to hear your opinion on this!

Kind regards,

Erik aka 4levels

kikoseijo commented 6 years ago

This is an easy one. 😜

The viewer its a query, you are correct, that returns the current logged in user.

now, from where, you can get the user > posts, user > comments, etc.

the viewer its just the name of que query.

You got it right.

kikoseijo commented 6 years ago

@4levels you got it implemented here https://github.com/kikoseijo/lumen-relay-boilerplate

Forgot to mention.

<?php

namespace App\GraphQL\Query;

use App\User;
use Folklore\GraphQL\Support\Query as BaseQuery;
use GraphQL;
use Illuminate\Support\Facades\Auth;
use Illuminate\Auth\Access\AuthorizationException;
use GraphQL\Type\Definition\ResolveInfo;
use GraphQL\Type\Definition\Type;

class ViewerQuery extends BaseQuery
{
    /**
     * @var array
     */
    protected $attributes = [
        'name'        => 'viewer',
        'description' => 'Get the current logged in user',
    ];

    protected function type()
    {
        return GraphQL::type('User');
    }

    /**
     * @param $root
     * @param $args
     * @param $context
     * @param ResolveInfo $info
     */
    public function resolve($root, $args, $context, ResolveInfo $info)
    {
        if(!$context || !$context->id){
            throw new AuthorizationException('Unauthorized');
            return null;
        }

        return $context; // User::find($context->id);
    }
}
4levels commented 6 years ago

Hi @kikoseijo,

I was actually figuring out how you did it in your excellent boilerplate project github.com/kikoseijo/lumen-relay-boilerplate Since I'm trying to avoid facades alltogether in my Lumen api, I managed to get everything I needed (I think) in my existing api project.
I see you related the User to Todos on the Eloquent level and in GraphQL via the Connection, but what if you just wanted to fetch the latest 10 News items that have no hard relation with the User?

As a side note: should I post this question in your project as well and refer to it here?

Kind regards,

kikoseijo commented 6 years ago

@4levels you what? Dont have clue, 🤔

Haven't done that approach, the implementation I did on production was to do with "only members" application.

From acquired experience I think the simples approach its the correct one, and a connection its the same as the query, build your query same you do in the connection and this library will do the rest for you.

From my perspective its just a different place to declare things and should work the same.

Maybe this is what you looking for, haven't touched this for few weeks.

<?php

namespace App\GraphQL\Query;

use Folklore\GraphQL\Support\Query;
use GraphQL\Type\Definition\ResolveInfo;
use GraphQL\Type\Definition\Type;
use GraphQL;
use App\Models\Tag;

class TagsQuery extends Query
{

    protected $attributes = [
        'name' => 'tagsForModel',
    ];

    public function type()
    {
        return Type::listOf(GraphQL::type('Tag'));
    }

    public function args()
    {
        return [
            'model' => [
                'type' => Type::nonNull(Type::string())
            ],
        ];
    }

    public function resolve($root, $args, $context, ResolveInfo $info)
    {

        $filterModel = array_get($args, 'model');

        $query = Tag::query();

        if ($filterModel) {
            $query->withType($filterModel);
        }
        return $query;
        // return $query->get();

    }
}

Notice you don't give back the ->get(); and this points you that at lower level the query must be filtered, etc..

Happy coding!

4levels commented 6 years ago

Hi @kikoseijo,

Thanks again for the very helpful hints you've been giving, especially the ->get() part changed at least some things. I've been trying with the master branch now as suggested by @dmongeau in issue #284 and copying over the Relay folder. This resulted in a lot of Exceptions related to the visibility of the args(), fields() and type() functions being declared as protected. The file Folklore\GraphQL\Console\GeneratorCommand is also missing, adding it resolves another Exception After adjusting wherever needed I get the same results: I can query the connection but the node remains empty. The PageInfo and eg. total do show that there is related stuff (the count is correct).

I'm still digging deeper to get this to work and keep you posted here.

Kind regards,

Erik

4levels commented 6 years ago

Hi @dmongeau & @kikoseijo,

I finally managed to get things working as a proof of concept. The culprit seems to be the $limit being set to 0 by default in Relay\Support\Traits\ResolvesFromQueryBuilder.php::resolve(). If I hardcode a limit of 10 (as a test) I'm finally getting edges with nodes and actual data (instead of null). I'm checking how this can be fixed atm..

Keep you posted!

Erik

kikoseijo commented 6 years ago

My branch of this package has relay merged with the lattest, and I think you need to know how to build the client queries... should not need to touch server side.

4levels commented 6 years ago

See PR's #305 and #306