Skip to content
This repository has been archived by the owner on Feb 10, 2019. It is now read-only.

Recursive input Objects do not work #313

Open
Artem-Schander opened this issue Mar 28, 2018 · 15 comments · May be fixed by #339
Open

Recursive input Objects do not work #313

Artem-Schander opened this issue Mar 28, 2018 · 15 comments · May be fixed by #339

Comments

@Artem-Schander
Copy link

I'm facing a strange behaviour. As soon as I define an input object to have a relation to itself (or to another input object that references the first one) I'm not able to execute my tests.

Correct me if I'm wrong but it should be possible to point a relation of an input object to one of its own kind. F.a. to save a nested navigation or hierarchical categories?!

@ralrom
Copy link

ralrom commented Mar 31, 2018

Did you take a look at this documentation?

If you have a recursive / circular type, you need to wrap your fields in a function for it to work

@Artem-Schander
Copy link
Author

Artem-Schander commented Mar 31, 2018

Thank you for the hint. I had no idea about this approach. Mainly because it kind of works in an older project, without that function wrap.
Anyway. Closing the issue then.

@Artem-Schander
Copy link
Author

Artem-Schander commented Apr 3, 2018

Sorry, but I'm getting the same result.
Here is a simplified version of my code:

<?php

namespace App\GraphQL\Types;

use GraphQL;
use GraphQL\Type\Definition\Type;
use Folklore\GraphQL\Support\Type as BaseType;

class CategoryInput extends BaseType
{
    protected $inputObject = true;

    public function fields()
    {
        return [
            'id' => [
                'type' => Type::int(),
            ],
            'name' => [
                'type' => Type::string(),
            ],
            'children' => [
                'type' => Type::listOf(MyTypes::category()),
            ],
        ];
    }
}

class MyTypes
{
    private static $category;

    public static function category()
    {
        return self::$category ?: (self::$category = GraphQl::type('CategoryInput'));
    }
}

It would be nice if someone could point out what I'm making wrong.
Thanks

@Artem-Schander Artem-Schander reopened this Apr 3, 2018
@Artem-Schander
Copy link
Author

Is there anybody out there?

@spawnia
Copy link

spawnia commented May 8, 2018

I am currently experiencing an issue with circular references in InputObjects. I have defined rules with my InputObjects, which causes inferRulesFromType and getInputTypeRules to run into an infinite loop.

I believe this issue can be resolved by looking at the $resulutionArguments, which do contain the actual input variables. At this point, the variables where already validated against the schema definition, right? So we can be sure the arguments structure matches with the fields of the defined InputObject. How about we only get the rules that correspond to an actual given value and stop looping once we iterated through the whole input args?

If no one is working on something like this currently, i might give it a shot and submit a PR. What do you think?

@spawnia
Copy link

spawnia commented May 9, 2018

While working on this, i found that we actually have two seperate issues going on here. I have opened another issue to detect rule definitions for circular referenced Input Objects that are impossible to resolve: #336

The second issue is one that lies within the way Laravel validations are used here. The current approach goes through the tree of InputObjects and infers an array of nested validation rules. Then, those rules and the complete input args are passed through the Laravel validator all at once.

One approach i could see would be to look at the depth of the provided input arguments and only infer the rules until that point. The other option would be to loop through the input arguments and run the validations on each iteration level, thus eliminating the need to infer the complete rules upfront.

What approach seems better in your opinion, any Pros/Cons?

@spawnia spawnia linked a pull request May 16, 2018 that will close this issue
@bytebrain
Copy link

bytebrain commented Sep 16, 2018

I used this:

CategoryType.php

<?php
namespace App\GraphQL\Type;

use Folklore\GraphQL\Support\Facades\GraphQL;
use GraphQL\Type\Definition\Type;
use Folklore\GraphQL\Support\Type as GraphQLType;

use App\Models\Category;

class CategoryType extends GraphQLType
{
    protected $attributes = [
        'name' => 'Category',
        'description' => 'A category',
        'model' => Category::class,
    ];

    public function fields()
    {
        return [
            'id' => [
                'type' => Type::nonNull(Type::int()),
                'description' => 'The id of a category'
            ],
            'name' => [
                'type' => Type::string(),
                'description' => 'The title of a category'
            ],
            'parent_id' => [
                'type' => Type::int(),
                'description' => 'The parent category id of a category'
            ],
            // field relation to model user_profiles
            'children' => [
                'type' => Type::listOf(GraphQL::type('Category')),
                'description' => 'The child categories of a category',
                'always' => ['id', 'name'],
            ]
        ];
    }
}

My model:
Category.php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

class Category extends Model
{
	protected $table = 'categories';
	protected $fillable = ['parent_id', 'name'];
	public $timestamps = false;

	public function children()
	{
		return $this->hasMany('App\Models\Category', 'parent_id', 'id');
	}
}

@spawnia
Copy link

spawnia commented Sep 16, 2018

Since my fix for this is not getting merged, and this Repo is apparently unmaintained i jumped ship. I can warmly recommend https://lighthouse-php.com/, we are actively developing it and it is a breeze to develop with.

And yeah, it does support recursive nested validation. You can even insert nested Mutations spanning multiple models and their relations, all without writing any PHP.

@KVSocial
Copy link

Since my fix for this is not getting merged, and this Repo is apparently unmaintained i jumped ship. I can warmly recommend https://lighthouse-php.netlify.com/, we are actively developing it and it is a breeze to develop with.

And yeah, it does support recursive nested validation. You can even insert nested Mutations spanning multiple models and their relations, all without writing any PHP.

Appreciate your effort on lighthouse; question though how much work is switching from this laravel graphQL package to yours? because to me it looks like a shit ton of work to switch on bigger project.

@Artem-Schander
Copy link
Author

@spawnia
lighthouse looks really promising.
But like KVSocial said, it's for sure not that easy to migrate.
However, for future projects I'll definitely give it a try.

@spawnia
Copy link

spawnia commented Sep 17, 2018

It depends on how specialized your Queries are. If you do mostly CRUD and a lot of Eloquent, it is a breeze. If not, you will have to dig in a little deeper, but after a while i think it will be way easier to add new features and to maintain the project.

You can use the built in printer of webonyx/graphql-php to get your schema as SDL, which will be a great starting point. http://webonyx.github.io/graphql-php/reference/#graphqllanguageprinter

You can try it for a small part of your schema and see if you like it :)

@mfn
Copy link

mfn commented Sep 18, 2018

because to me it looks like a shit ton of work to switch on bigger project

It's already ton of work a small project as I'm having.

It depends on how specialized your Queries are. If you do mostly CRUD and a lot of Eloquent, it is a breeze. If not, you will have to dig in a little deeper

This. I tried to make a quick win here (max 2 hours to see how far I get) but I mostly do not use CRUD at all; it's all specialised, custom resolvers all the way and also the customization I needed for this library (error reporting isn't very satisfying, took some time to get it straight) I have to go over all again.

But, eventually: I will have to do it. Folklore is pretty much dead

@spawnia
Copy link

spawnia commented Sep 18, 2018

@mfn Yeah, error handling is quite a bit different in GraphQL. You can not really abort on the first error, since you might have another field that worked and should return data.

I did a major rewrite on how Lighthouse does Error handling, you might want to check out the newest version. I am quite interested in how you did it as well!

@mfn
Copy link

mfn commented Sep 18, 2018

@spawnia my biggest grief was that all exceptions where reported in the GraphQL result payload but they were not logged anywhere => completely useless:

  • got a DB error => got a (anonymized) response back but the exception was not logged
  • also in tests, they never aborted with the internal exceptions but just returned a 200/400 payload (catchable but wrong)

That was a major pain issue because exception not being logged means it does not go through your regular exception handler channels which means they are not properly e.g. reported to 3rd party Services (like Sentry).

I solved it with a custom error handler, a re-implementation of folklores own way how to dispatch errors and a fallback in Exception\Handler::render(). A sh.. ton of work :-) Not really "much" work but took ages to figure it out, get it right, etc.

Will give my feedback about lighthouse when I finally can make time for the switch :-)

@spawnia
Copy link

spawnia commented Sep 18, 2018

webonyx/graphql-php does a good job of collecting the errors. I made it so our error handler sends all Exceptions through a pipeline that works just like middleware. You can register any number of handlers that may rethrow, log, reformat or do whatever with the Errors.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants