The root_value of ExecutionContext.execute_operation should not be None

[ethe]

Usually I want to use the custom method binded to GraphQLObjectType, such as:

# use strawberry for example
import strawberry


@strawberry.type
class User:
    name: str
    age: int


@strawberry.type
class Query:
    @strawberry.field
    def user(self, info) -> User:
        return self.get_user()

    def get_user(self):
        return User(name="Patrick", age=100)


schema = strawberry.Schema(query=Query)

Unfortunately, the self would always be None cause the root_value in ExecutionContext.execute_operation would be setted to None if it is the root node Query. I think modifying it as below is fine:

def execute_operation(
        self, operation: OperationDefinitionNode, root_value: Any
    ) -> Optional[AwaitableOrValue[Any]]:
        """Execute an operation.

        Implements the "Evaluating operations" section of the spec.
        """
        type_ = get_operation_root_type(self.schema, operation)
        if not roo_value:
            root_value = type_

Then we can use the custom method of GraphQLObjectType. And it not leads any problem I think.

[Cito]

Thank you for this suggestion @ethe - I think I understand your use case. However, I'm very reluctant to change anything that breaks the compatibility with the GraphQL.js by introducing different or additional behavior. Maybe some people detect that they are at root by comparing the object with None.

My suggestion is to determine the type automatically before running the query, something like this:

root_value = get_operation_root_type(schema, parse(query).definitions[0])

And then passing it explicitly as the root_value argument when running the query.

[ethe]

Passing None to self is not in good taste I think, cause it is conflict with common sense. But it is sensible if root_value is used in stand alone function such as

schema = GraphQLSchema(GraphQLObjectType("Query", {"test": GraphQLField(
    GraphQLString,
    args={
        "aStr": GraphQLArgument(GraphQLString),
        "aInt": GraphQLArgument(GraphQLInt),
    },
    resolve=lambda source, info, **args: dumps([source, args]))}))

So I think maybe we need to treat method resolver and function resolver in different ways.

[KaySackey]

Resolvers are supposed to be staticmethods, so there is no self to pass None or any other value to.

That said, this design choice was inherited from graphql.js (where it makes sense due to JavaScript generally not using class syntax to create queries, and the problems with binding this in JS).

It makes less sense in Python, and isn’t really part of the graphql spec but this lib is attempting to be a direct port from the JavaScript so it inherited some odd semantics: static resolvers, promises, heavy use of lambdas, etc.

[ethe]

@KaySackey Exactly, I am considering write a wrapper for graphql-core to make it friendlier to Python.

[Cito]

@ethe Wouldn't it work when you pass bound methods as resolvers to graphql-core?

[ethe]

@Cito There are not good opportunities to initialize the Query object. Neither graphene nor strawberry support that.

[Cito]

I think the way forward then is to discuss this as a Graphene or Strawberry issue and if it turns out it really cannot be solved without additional support of GraphQL-core, then reopen an issue here.

Note that it is already possible to use a custom ExecutionContext class in GraphQL-core where you can overwrite the execute_operation method as you like.

Closing this for now as currently don't think this is something that should be addressed by GraphQL-core.

[ethe]

I just create another library and rewrite the whole of type system design to avoid those weird behaviors: https://github.com/ethe/pygraphy, cause it is really hard to walk around. The type object should be a metaclass in a more pythonic way, and it is natural to realize the user custom type initialized from type metaclasses.

[Cito]

@ethe isn't that exactly what Graphene is doing?

[ethe]

Sorry, I do not think any library which is based on graphql-core(-next) can totally solve this issue. To those class-based model approach implementation such as Graphene and Strawberry, make type class be a python meta class is the best way to solve those weird behaviors, it is a more pythonic way. However, it can not just be inherited from graphql.js, cause Javascript have neither class nor meta class.

[ethe]

@Cito The root cause is, core uses object-based type declaration system(type classes are python class and user custom types are class instances), it is counterintuitive in Python. It is natural that as a higher-ranked type, type class should be metaclass, then we got all custom types becomes python class, and object just be an object in Python. Everything is perfectly matched.

[ethe]

Let me compare with the type declaration design between GraphQL-Core and Pygraphy.

The type class of core is just python class

class GraphQLObjectType:
    pass

And user custom type is a python instance.

CustomFoo = GraphQLObjectType(
    name='CustomFoo',
   fields={}
)

See where the inharmony is? the CustomFoo is not a python class so it can not bind any method on it. Every solution attempts to walk around is ugly and not pythonic. Pass an unbounded method as a resolver field? No, stop doing that. Initialize the model object and pass bounded method in? Where and when can I initialize it? And If I want to keep the behavior of each request having their standalone instance, I can not see a way to realize it.

The type declaration design of Pygraphy like the below example:

class GraphQLObjectType(type):
    pass

class GraphQLObject(metaclass=GraphQLObjectType):
    pass

class UserCustomFoo(GraphQLObject):
    pass

UserCustomFoo inherits from a common Object class which is bound to a meta class. There is also an initialization here, but it happens in class defining rather than object initializing.

Obviously, we can bind a resolver as a python method to the UserCustomFoo naturally and all things obey the intuition of Python developing.

[Cito]

I agree, it makes sense to experiment with other approaches and create a library that is Pythonic from the start, instead of the approach of copying GraphQL.js 1:1 and then putting a more Python wrapper on top. But I think GraphQL-core should stay what it is, and this should be done in other projects.

[ethe]

@Cito That's the reason I wrote Pygraphy as a supplement to graphql-core-next.