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

Type Difficult to use, not ORM friendly #640

Open
1 of 3 tasks
vt-rc opened this issue Oct 13, 2024 · 4 comments
Open
1 of 3 tasks

Type Difficult to use, not ORM friendly #640

vt-rc opened this issue Oct 13, 2024 · 4 comments
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@vt-rc
Copy link

vt-rc commented Oct 13, 2024

Feature Request Type

  • Core functionality
  • Alteration (enhancement/optimization) of existing feature(s)
  • New behavior

Description

I am trying to transit from graphene to strawberry, but it is very troublesome. I am surprise that the things that Graphene does so well are not adopted by strawberry.

My main issues are
(1) Model based types doesn't auto discover relationship and just link up
- in graphene, DjangoObjectType does this extremely well
(2) Not very convenient to support custom orm fields
- graphene also does this extremely well, with conversion
(3) Not easy to add decorator on top of type, which are already wrapped in strawberry_django.type.

As Django developer, we are more ORM first, but Strawberry is probably too dataclass first, and leads to a lot of duplicate work and declaration. Ideally I should define ORM first and use it to generate type.

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

Hi @vt-rc ,

Indeed there are some places where we can do better, but some are not that straightforward. Let me try to address each of your concerns:

(1) Model based types doesn't auto discover relationship and just link up

  • in graphene, DjangoObjectType does this extremely well

This is a topic that has come up numerous times.

Graphene solves this with a registry, but there's a gotcha with it that has hit some people already (me included): When you have 2 or more types for a given model, which one should be used for the relationship? It's been a while since I last worked with graphene so I don't remember if it will use the first one (because it will avoid overriding the registry) or the last one (because a new one will override the existing one).

We already discussed this on discord, and the solution that came up was to have an extra skip_registry argument in the type() decorator, and raise an error if 2 or more types are found for the same model without the non-main ones skipping the registry. I just need to implement that :)

(2) Not very convenient to support custom orm fields

  • graphene also does this extremely well, with conversion

Right now you can add your custom field to the field_type_map. I agree that it is not obvious and probably having a public API + some documentation would be even better! Could you maybe open an issue about this? This one should be simple enough to tackle very soon

(3) Not easy to add decorator on top of type, which are already wrapped in strawberry_django.type.

Not sure what you mean here. Could you explain this issue a bit further?

@vt-rc
Copy link
Author

vt-rc commented Oct 13, 2024

Thanks for the quick response. I really appreciate it.

(1) So do you mean the feature to autolink types base on orm relationship is available now? And just not the skip_registry feature? Or you still need to implement both registry and skip_registry? I cannot find the documentation on how to do this on strawberry.

so (3)
In graphene, We often insert additional field with resolver, or generic helper method, to the type, when certain field of the model is detected. This is easy to do because I just add a decorator on top of DjangoObjectType. In strawberry, Type are wrapper I think, and it's not very intuitive on how I can do this. I guess I am just not very familiar with this style of coding.

@bellini666
Copy link
Member

(1) So do you mean the feature to autolink types base on orm relationship is available now? And just not the skip_registry feature? Or you still need to implement both registry and skip_registry? I cannot find the documentation on how to do this on strawberry.

Not yet, but I'm planning on adding it in the near future.

In graphene, We often insert additional field with resolver, or generic helper method, to the type, when certain field of the model is detected. This is easy to do because I just add a decorator on top of DjangoObjectType. In strawberry, Type are wrapper I think, and it's not very intuitive on how I can do this. I guess I am just not very familiar with this style of coding.

You can add additional fields with resolvers like this: https://strawberry.rocks/docs/django/guide/resolvers

You can even define those as a @property in your model and use field: strawberry.auto in there, as long as the @property has a proper type annotation.

If you are using the optimizer extension, which I highly recommend you to use, strawberry-django also provides a @model_property, which functions exactly like a @property, but can receive hints for optimization: https://strawberry.rocks/docs/django/guide/optimizer#optimization-hints-on-model-modelproperty

Not sure if that answers the questions, please let me know

@bellini666 bellini666 added documentation Improvements or additions to documentation question Further information is requested labels Oct 19, 2024
@vt-rc
Copy link
Author

vt-rc commented Nov 7, 2024

Thank you for your response. I really appreciate it.
Yes I know I can add additional field with resolvers like presented in the guide

My problem is that I want to add them using another decorator that add the fields and resolver base on custom Django ORM field. This way I can define my logic base on custom ORM field once, and reuse those logic in every model and services. My purpose is to remove as much manual definition effort as possible.

It's probably doable to nest another decorator on top @strawberry_django.type(models.Model), but its probaby messy and not obvious how to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants