strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
MIT License
391 stars 115 forks source link

Tweak to improve DX for newcomers (re-exported strawberry type) #462

Open thclark opened 5 months ago

thclark commented 5 months ago

Feature Request Type

Description

When I was working with the documentation improvements the other week, one area where I realised I was struggling was the difference between @strawberry.type and @strawberry_django.type.

See my comments in the code below (extracted from that featured in the docs):

import strawberry
import strawberry_django
from strawberry import auto
from .. import model

# It makes sense that a Type is wrapped as a type
@strawberry_django.type(models.Fruit)
class FruitType:
    id: auto
    name: auto
    category: auto
    color: "ColorType"

# It makes sense that a Type is wrapped as a type
@strawberry_django.type(models.Color)
class ColorType:
    id: auto
    name: auto
    fruits: list[
        FruitType
    ]

# It doesn't make sense that a Query is wrapped as a type
@strawberry.type
class FruitQueries:
    fruits: list[FruitType] = strawberry_django.field()
    # (it's also not quite obvious how strawberry_django.field() works here but that's for another issue I guess)

Proposed Solution

What about, within strawberry-django, we do:

from strawberry import type as query
# Or possibly something similar but with different docstring explaining what this is for

Then the last section of the code above looks like:

# It makes sense that a Query is wrapped as a query
@strawberry_django.query
class FruitQueries:
    fruits: list[FruitType] = strawberry_django.field()

I think this solution has the following benefits:

Effort required

Less than an hour. I'm happy to make this PR myself and update documentation to match if my suggestion has the blessing of the maintainers, no breaking changes required.

Upvote & Fund

Fund with Polar

bellini666 commented 5 months ago

Hrm, I get what you mean here from a new user perspective, specially for someone new to strawberry that usually don't know that Query and Mutation are types as well.

I somewhat like this proposal, but I'm not totally sure it brings value enough instead of improving the documentation and ensuring the user knows the difference between strawberry_django.type and strawberry.type.

What do you think about this @patrick91 ?

patrick91 commented 5 months ago

I'm -1 for this 😊

A GraphQL Query type is a standard type, what makes it special is it being set on strawberry.Schema, I understand that might be confusing and we should make it less confusing maybe with docs, but I don't think a new decorator will make it less confusing

thclark commented 4 months ago

OK, so -2 on renaming the decorator.

We could elaborate in the docs that Queries and Mutations are also types and that's why they're wrapped in a type decorator, but still using the type decorator from a different, underlying, library is definitely a point of hangup for new users.

Could it work to simply use @strawberry_django.type() with no model argument?

bellini666 commented 4 months ago

We could elaborate in the docs that Queries and Mutations are also types and that's why they're wrapped in a type decorator, but still using the type decorator from a different, underlying, library is definitely a point of hangup for new users.

I do agree that Query and Mutation also being types is a source of confusion between users. It took myself some time to understand this as well back when I started working with graphene =P

Maybe we could acomplish this by adding docs on strawberry itself? Specially because we are soon going to display strawberry-django's docs as a subsection of strawberry docs, which might help alleviating the confusion.

Could it work to simply use @strawberry_django.type() with no model argument?

We would need to change strawberry_django.type() to allow it to not receive a model argument, but I would be against it as that would mean we would not be able to enforce typing on it correctly anymore.