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

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

Open
1 task done
thclark opened this issue Jan 26, 2024 · 4 comments
Open
1 task done

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

thclark opened this issue Jan 26, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@thclark
Copy link
Contributor

thclark commented Jan 26, 2024

Feature Request Type

  • Minor improvement by re-exporting something from strawberry

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:

  • Restricts the api for django users to strawberry_django to avoid digging immediately into the much bigger and more complex strawberry docs
  • Allows additional magic later (ie more than a simple type wrapper could be hooked up)
  • Gives an easier introduction in terms of the tutorial and setup code

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

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@bellini666
Copy link
Member

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 ?

@bellini666 bellini666 added the enhancement New feature or request label Jan 27, 2024
@patrick91
Copy link
Member

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
Copy link
Contributor Author

thclark commented Feb 14, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants