strawberry-graphql / strawberry

A GraphQL library for Python that leverages type annotations 🍓
https://strawberry.rocks
MIT License
4.03k stars 534 forks source link

Improve how we fetch the type for fields with resolvers #396

Open patrick91 opened 4 years ago

patrick91 commented 4 years ago

To define a type for a field we use the approach used by dataclasses:

@strawberry.type
class Q:
    x: int

But we also allow to use strawberry.field to pass a resolver:

def get_x() -> int:
    return 123

@strawberry.type
class Q:
    x: int = strawberry.field(resolver=get_x)

Now, we could allow to omit the type on the field, like this:

def get_x() -> int:
    return 123

@strawberry.type
class Q:
    x = strawberry.field(resolver=get_x)

And we'll use the type coming from the resolver, like we do when using @strawberry.field as a decorator.

This shouldn't be too difficult to achieve, I think the only complexity might be that removing the type might hide the field from the dataclasses fields, thus having to find it manually here: https://github.com/strawberry-graphql/strawberry/blob/master/strawberry/types/type_resolver.py#L224-L235

Currently we also allow to use resolvers without type hints:

def get_x():
    return 123

@strawberry.type
class Q:
    x: int = strawberry.field(resolver=get_x)

I think we should keep this, and maybe just log a warning, which might be useful for debugging.

We also have to consider these two cases where things might be wrong:

  1. Both places are missing type hints
def get_x():
    return 123

@strawberry.type
class Q:
    x = strawberry.field(resolver=get_x)

I'm not 100% what happens in this case, I guess the field isn't shown in the schema currently.

  1. Type hints don't match
def get_x() -> str:
    return '456'

@strawberry.type
class Q:
    x = strawberry.field(resolver=get_x)

Here we need to throw an error and let the user know that something is wrong with the type hints :)

Upvote & Fund

Fund with Polar

BryceBeagle commented 4 years ago

You might also want to consider what will happen with

def get_x() -> int:
    return 456

@strawberry.type
class Q:
    x: float = strawberry.field(resolver=get_x)

I'd assume the field's type will be used

patrick91 commented 4 years ago

@BryceBeagle That was supposed to be my second example, ironically I forgot the type hint 😂

I'd throw an error in this case

BryceBeagle commented 4 years ago

Well I specifically chose int and float because of PEP484

when an argument is annotated as having type float, an argument of type int is acceptable

Basically, anywhere something is annotated as accepting float, an int is supposed to be accepted

patrick91 commented 4 years ago

oh, that’s a good point. I wonder, to me it doesn’t seem explicit enough, and I’d prefer to thrown an error.

Happy to be change my opinion though, if there’s a use case for this. But I guess one could use an intermediate function for this :)

BryceBeagle commented 4 years ago

I agree -- I think as long as we include a note being pretty explicit that no number coercion occurs, explicit is better than implicit in this case

I can't think of any good use cases for the coercion to occur implicitly

patrick91 commented 4 years ago

@BryceBeagle yes, good idea! We can add some notes in the exception message too :)

BryceBeagle commented 4 years ago

Here are a couple more corner cases that we need to decide if they should be valid or not.

@strawberry.type
class Query:

    @strawberry.interface
    class Cheese:
        name: str
    @strawberry.type
    class Swiss(Cheese):
        canton: str

    def swiss_resolver() -> Swiss: ...
    def uuid_resolver() -> UUID: ...

    my_cheese: Cheese = strawberry.field(resolver=swiss_resolver)
    my_id: strawberry.ID = strawberry.field(resolver=uuid_resolver)