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

[RFC] Refactor GlobalVariables and resolver expressions #775

Closed
murtukov opened this issue Nov 6, 2020 · 1 comment · Fixed by #786
Closed

[RFC] Refactor GlobalVariables and resolver expressions #775

murtukov opened this issue Nov 6, 2020 · 1 comment · Fixed by #786
Labels
Milestone

Comments

@murtukov
Copy link
Member

murtukov commented Nov 6, 2020

Q A
Bug report? no
Feature request? no
BC Break report? no
RFC? yes
Version/Branch 1.0 (master)

Some of the proposed changes are BC-break.

List of proposed changes

1. Rename GlobalVariables class.

GlobalVariables is a service passed to every generated GraphQL type. It contains all necessary services and references to other types to configure a GraphQL schema. The name GlobalVariables doesn't really explain anything, I suggest to rename it to better reflect it's purpose.

Suggested name: GraphQLServices
Other possible names: SchemaServices, TypeServices, TypeHelper

2. Move ConfigProcessor into GlobalVariables

Every generated type also gets ConfigProcessor object, which is only used in types and nowhere else. If GlobalVariables contains all necessary services for types, then why not also moving the ConfigProcessor inside it?

Before:

final class QueryType extends ObjectType implements GeneratedTypeInterface
{
    public const NAME = 'Query';
    
    public function __construct(ConfigProcessor $configProcessor, GraphQLServices $services)
    {
        $config = [
            'name' => self::NAME,
            'fields' => fn() => [
                'post' => [
                    'type' => Type::string(),
                    'resolve' => fn() => "Hello, World!",
                ],
            ],
        ];

        parent::__construct($configProcessor->process($config));
    }
}

After:

final class QueryType extends ObjectType implements GeneratedTypeInterface
{
    public const NAME = 'Query';
    
    public function __construct(GraphQLServices $services)
    {
        $config = [
            'name' => self::NAME,
            'fields' => fn() => [
                'post' => [
                    'type' => Type::string(),
                    'resolve' => fn() => "Hello, World!",
                ],
            ],
        ];

        parent::__construct($services->get('configProcessor')->process($config));
    }
}

3. Add shortcuts into GraphQLServices

Add methods for frequently used services, like typeResolver, resolverResolver and mutationResolver

Before:

final class QueryType extends ObjectType implements GeneratedTypeInterface
{
    public const NAME = 'Query';
    
    public function __construct(GraphQLServices $services)
    {
        $config = [
            'name' => self::NAME,
            'fields' => fn() => [
                'users' => [
                    'type' => $services->get('typeResolver')->resolve('User'),
                    'resolve' => function ($value, $args, $context, $info) use ($services) {
                        return $services->get('resolverResolver')->resolve(["get_users", [0 => $args, 1 => $value]]);
                    },
                ],
                'createUser' => [
                    'type' => $services->getType('User'),
                    'resolve' => function ($value, $args, $context, $info) use ($services) {
                        return $services->get('mutationResolver')->resolve(["create_user", [0 => $args, 1 => $value]]);
                    },
                ],
            ],
        ];

        parent::__construct($services->get('configProcessor')->process($config));
    }
}

After:

final class QueryType extends ObjectType implements GeneratedTypeInterface
{
    public const NAME = 'Query';
    
    public function __construct(GraphQLServices $services)
    {
        $config = [
            'name' => self::NAME,
            'fields' => fn() => [
                'users' => [
                    'type' => $services->getType('User'),
                    'resolve' => function ($value, $args, $context, $info) use ($services) {
                        return $services->query("get_users", $args, $value);
                    },
                ],
                'createUser' => [
                    'type' => $services->getType('User'),
                    'resolve' => function ($value, $args, $context, $info) use ($services) {
                        return $services->mutation("create_users", $args, $value);
                    },
                ],
            ],
        ];

        parent::__construct($services->processConfig($config));
    }
}

In this case arrow functions can be used:

final class QueryType extends ObjectType implements GeneratedTypeInterface
{
    public const NAME = 'Query';
    
    public function __construct(GraphQLServices $services)
    {
        $config = [
            'name' => self::NAME,
            'fields' => fn() => [
                'users' => [
                    'type' => $services->getType('User'),
                    'resolve' => fn($value, $args) => $services->query("get_users", $args, $value),
                ],
                'createUser' => [
                    'type' => $services->getType('User'),
                    'resolve' => fn ($value, $args) => $services->mutation("create_users", $args, $value),
                ],
            ],
        ];

        parent::__construct($services->processConfig($config));
    }
}

So now instead of this:

$services->get('mutationResolver')->resolve(["get_users", [0 => $args, 1 => $value]])

we have this

$services->mutation("get_users", $args, $value) // variable-length argument list

The signature is also changed to be simpler.

4. Use QueryResolver instead of ResolverResolver

We have 2 expression resolver functions: resolver and mutation. If we are using the word mutation, which is a type
of resolver, then I suggest renaming resolver into query to be more like a counterpart of mutation. It will also
make more sense in YAML configs (no tautology resolve: resolver):

Before:

Query:
    type: object
    config:
        fields:
            users:
                type: String
                resolve: '@=resolver("get_users", [args, info])'

After:

Query:
    type: object
    config:
        fields:
            users:
                type: String
                resolve: '@=query("get_users", args, info)'

The signature is also more simple now (no arrays as suggested in section 3)

It also includes renaming of the ResolverInterface into QueryInterface

5. Make the expression prefix configurable

In my projects I would like to be able to change the expression trigger. The symbols @= at the beginning of a string
require it to be wrapped in quotes. If I change the trigger symbol to $ for example, then I don't have to wrap
expression strings into quotes:

Before:

resolve: '@=mutation("get_users", args, info)'

After:

resolve: $mutation("get_users", args, info)

Or just use @:

resolve: '@mutation("get_users", args, info)'

A better alternative to GraphQLServices would be of course a separate injection of only required services into each of generated types, instead of injecting a container, which is considered a bad practice, but I am currently not sure how to implement it.

@akomm
Copy link
Member

akomm commented Nov 6, 2020

Also to note, currently injecting stuff at runtime via globalVariable requires you to have services set public. For constructor injection you would need to make all the generated types available as services adjusting container definitions in compiler pass. I can't quite figure out the implication doing so... But if it would be possible it would be great...

Agree on other points (smiles ResolverResolver made me grin a bit once)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment