strawberry icon indicating copy to clipboard operation
strawberry copied to clipboard

[WIP] Support custom fields

Open nrbnlulu opened this issue 3 years ago • 7 comments
trafficstars

Description

Disclaimer: This PR was started as a fix for #920 and expanded to refactoring of the strawberry type system.

Motivation:

While walking threw the code I have found that being able to extend StrawberryField was pretty hard for these reasons:

  1. the field evaluation process relied mostly on functions (namely _get_fields).
  2. StrawberryField was inheriting dataclasses.Field and that IMO unnecessary burden, results namespace clashes, tights strawberry to dataclasses too much and finally it is not a dataclass like many other parts of strawberry.
  3. generic types was evaluated in a very complex way (copy_with) and it is not clear at a first glance when a type is already evaluated or not,
  4. Integrations (like pydantic) are building lots of things on their own and some functionalities that strawberry provides or not allows might not be covered in the integration, as well as they have lots of redundant code that mimics what strawberry-core already does.
  5. Usage of @property was quite extensive and that is a runtime burden and makes hard to deduce what will a value be at a given point.
  6. there are probably more things I forgot but the general idea was to padronize the way strawberry does things and rely more on classes than on functions to allow extending.

Implementation details:

Generics:

  • if a class inherits Generic it would be created as class TemplateTypeDefinition(TypeDefinition) This class can generate a whole new "StrawberryObject" based on __args__. the generator is TemplateTypeDefinition.generate(<types>). it is lunched by the class __getitem__ that defined in StrawberryMeta, so i.e Apple[int] is equivalent to Apple._type_definition.generate((int, )).
  • I introduced new validate abstract method to StrawberryType it is used to determine generic unions.
    NOTE: With the new API for generating templates this might not even be necessary because users would be able to do this:
@strawberry.type
class A(Generic[T]):
   a: T

@strawberry.type
class SomeType():
   @strawberry.field
   def union_generic(self) -> Union[A[int], A[str]]
       return A[int](1)

though this would break existing code.

  • generics are now cached by __args__ signature and every type is created just once. This is tested using timeit.

Scalars:

I removed ScallarWrapper as it made issued for me with the new validate method of StrawberryType, every scalar now is just a ScalarDefinition, I would consider renaming this to StrawberryScalar.

strawberry.ID

strawberry.ID is no longer a plain NewType but a ScalarDefinition using implementation from graphql-core, I saw that is is a 'TODO' and also it did issues with validate() of generic unions.

pydantic:

I have also refactored pydantic integration, and it is no longer building things on it's own, it connects to hooks from strawberry-core to append and modify fields.

StrawberryObject:

strawberry object is what will lay behind @strawberry.type and it is extending the StrawberryMeta class to evaluate generics as explained above. This has the benifit of duck-typing _type_definition and allows integrations to connect to strawberry type creation system. The workflow is like this:

  1. StrawberryObject creates a new type from the decorated class that extends StrawberryObject.
  2. StrawberryObject calls a customizable function to create the TypeDefinition (to support extending by integrations).
  3. TypeDefinition will find all the fields in the class including pre-evaluated dataclasses fields.
  4. TypeDefinition will create a StrawberryField for each field (integrations can define their own StrawberryField extended and it will be created instead).
  5. TypeDefinition will call __pre_dataclass_creation__ to allow integrations to add or modify fields.
  6. dataclass is created and the rest is history.

As you can see this PR won't only allow custom fields but also custom StrawberryObject and TypeDefinition the pydantic integration can set an example for this.


Please contact me if You want more information, Thank you @patrick91, @BryceBeagle for your valuable time and patience.

Types of Changes

  • [x] Core
  • [ ] Bugfix
  • [x] New feature
  • [ ] Enhancement/optimization
  • [x] Documentation

Issues Fixed or Closed by This PR

  • #920

Checklist

  • [ ] My code follows the code style of this project.
  • [x] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] I have read the CONTRIBUTING document.
  • [x] I have added tests to cover my changes.
  • [ ] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

tests

  • [x] renamer field
  • [x] type changer field
  • [x] argument modification field
  • [x] override get_result field
  • [ ] batched fields.
  • [ ] field "__post_init__"
  • [x] not supported resolver assertion
  • [x] not supported evolvable
  • [ ] StrawberryField was initialized with origin but with no python name assertion.
  • [ ] test validate generic union with StrawberryObject

nrbnlulu avatar Aug 22 '22 19:08 nrbnlulu

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release will try to address #1100.

New features:

  • allow overriding StrawberryField's
    • get_result - for overriding the execution of resolvers.

Here's the preview release card for twitter:

Here's the tweet text:

🆕 Release (next) is out! Thanks to Nir for the PR 👏

Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)

botberry avatar Aug 22 '22 19:08 botberry

Also, I have rename _type_defenition to __strawberry_definition__ I hope you will forgive me not putting this in another PR, though _type_defenition is kinda cryptic :upside_down_face: ...

nrbnlulu avatar Aug 24 '22 07:08 nrbnlulu

re: @patrick91

is there a way we can break it into multiple PRs?

Unfortunately It would be very hard for me. most of what I made is ought to co-operate with each other. It would take a lot of my time, I hope you can understand :disappointed: . Though if you want me to walk you threw my changes via a phone call or discord voice chat I would be more than happy.

re: @patrick91

I've seen some changes that don't seem strictly related to the custom field work

I know, but the previous design really made it hard to extend stuff, because most of it relied on functions. also you can see this as a refactoring PR and I will reserve the custom field implementation for another PR.

nrbnlulu avatar Sep 01 '22 17:09 nrbnlulu

@nrbnlulu ok, I'll reach out to you in the next few days 😊 thanks for making time to go through the code

patrick91 avatar Sep 01 '22 18:09 patrick91

@nrbnlulu ok, I'll reach out to you in the next few days blush thanks for making time to go through the code

It was my pleasure, I learned a lot.

nrbnlulu avatar Sep 01 '22 18:09 nrbnlulu

@patrick91 Any updates on this one?

nrbnlulu avatar Sep 21 '22 06:09 nrbnlulu

@nrbnlulu not yet sorry, I'm travelling for conferences so I don't have a lot of focused time 😊

patrick91 avatar Sep 21 '22 07:09 patrick91