prototype: object type extending

[Kitefiko]

Prototype for https://github.com/strawberry-graphql/strawberry/issues/2605

[botberry]

Hi, thanks for contributing to Strawberry 🍓!

We noticed that this PR is missing a RELEASE.md file. We use that to automatically do releases here on GitHub and, most importantly, to PyPI!

So as soon as this PR is merged, a release will be made 🚀.

Here's an example of RELEASE.md:

Release type: patch

Description of the changes, ideally with some examples, if adding a new feature.

Release type can be one of patch, minor or major. We use semver, so make sure to pick the appropriate type. If in doubt feel free to ask :)

Here's the tweet text:

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

Get it here 👉 https://beta.strawberry.rocks/release/(next)

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 97.05882% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.57%. Comparing base (f15bcc7) to head (41a44b1).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3379   +/-   ##
=======================================
  Coverage   96.57%   96.57%           
=======================================
  Files         524      525    +1     
  Lines       33630    33651   +21     
  Branches     5578     5580    +2     
=======================================
+ Hits        32479    32500   +21     
+ Misses        915      914    -1     
- Partials      236      237    +1     

[codspeed-hq[bot]]

CodSpeed Performance Report

Merging #3379 will not alter performance

_{Comparing Kitefiko:object-extension (41a44b1) with main (f15bcc7)}

Summary

✅ 13 untouched benchmarks

[Kitefiko]

Just rebased and adjusted so solution is no longer outdated - original_type_annotations.

[bellini666]

I like where this is going...

Checking it today I'm wondering if we should alllow a list of type extensions instead of just one.

Then when doing, field = extension.on_field you just replace it with

for extension in extensions:
    field = extension.on_field(field)

And so on.

This would allow for much more customization.

What do you think about this @patrick91 ?

[patrick91]

Checking it today I'm wondering if we should alllow a list of type extensions instead of just one.

yup, we do this for fields already 😊

[Kitefiko]

Then when doing, field = extension.on_field you just replace it with

for extension in extensions:
    field = extension.on_field(field)

This would allow for much more customization.

There are 2 reasons why I didn't do that.

• Reduce performance hit to minimum - no looping (this might not matter tho)
• Keep it more generic. Since you already have access to lower level and thus can modify entire object (including fields), it seemed redundant to enforce this via library

You could do something like

class MyTypeExtension(TypeExtension):
    def __init__(self, field_extensions: Iterable[...]):
        self._field_extensions = field_extensions

    def on_field(self, field: dataclasses.Field[Any]):
        for ext in self._field_extensions:
            field = ext.apply(field)
        return field

What do you guys think?

Also could you explain a little more how you envision the usage of context manager styled hooks @bellini666 ?

[bellini666]

Then when doing, field = extension.on_field you just replace it with

for extension in extensions:
    field = extension.on_field(field)

This would allow for much more customization.

There are 2 reasons why I didn't do that.

• Reduce performance hit to minimum - no looping (this might not matter tho)
• Keep it more generic. Since you already have access to lower level and thus can modify entire object (including fields), it seemed redundant to enforce this via library

IMO I think it is still worth it. This only runs once, and the performance hit here should be insignificant.

Also could you explain a little more how you envision the usage of context manager styled hooks @bellini666 ?

Hrm, I think the code that I mentioned changed now, but I was thinking in something like this: https://github.com/strawberry-graphql/strawberry/blob/main/strawberry/extensions/base_extension.py#L27 . With that you can have some code that executes "before" and "after" without having to define 2 functions for that.

But I don't think that makes sense for the on_field for example, as it is supposed to be able to actually change the field.

[Kitefiko]

Then when doing, field = extension.on_field you just replace it with

for extension in extensions:
    field = extension.on_field(field)

This would allow for much more customization.

There are 2 reasons why I didn't do that.

• Reduce performance hit to minimum - no looping (this might not matter tho)
• Keep it more generic. Since you already have access to lower level and thus can modify entire object (including fields), it seemed redundant to enforce this via library

IMO I think it is still worth it. This only runs once, and the performance hit here should be insignificant.

True that, some more things to consider between single and multi TypeExtension per object. With one TypeExtension you will be allowed to subclass StrawberryObjectDefinition. This would allow to add custom methods, custom data as attrs, and use isinstance on cls.__strawberry_definition__. With multiple I don't really see a way how to subclass StrawberryObjectDefinition so the only "modifiable" thing might be metadata?

I personally like single TypeExtension mainly due to bigger freedom and ability to directly modify __strawberry_definition__. With multiple approach I would be back to monkey patching. One could get multiple approach functionality from single approach one, but not vice versa.

Also could you explain a little more how you envision the usage of context manager styled hooks @bellini666 ?

Hrm, I think the code that I mentioned changed now, but I was thinking in something like this: https://github.com/strawberry-graphql/strawberry/blob/main/strawberry/extensions/base_extension.py#L27 . With that you can have some code that executes "before" and "after" without having to define 2 functions for that.

But I don't think that makes sense for the on_field for example, as it is supposed to be able to actually change the field.

Thanks for clarifying this. I do like context manager approach here too, after figuring out how it works. I might have added this at the latest update but, after_process currently returns cls. I added this due to how pydantic wrapper replaces original class with new. I think however that that is very unlikely scenario so context manager is better, unless someone thinks otherwise.

I was playing with integration into django library and thing popped up: should on_field hook have additionally also origin info from _get_fields.origins that is gather from all bases?

[Kitefiko]

The main open point I see is wether we want to support one or multiple type extensions on the same type.

Yes. This is currently blocker right now. I would really appreciate for more people to weight in.

My initial draft aimed for the support of multiple extensions, however I am open to other opinions and curious to hear why you chose to take this approach.

I have touched on this in previous comment but the main points IMO are:

Single

• Ability to hook into objects' field creation
• Ability to directly inherit and modify StrawberryObjectDefinition used for __strawberry_definition__. This allows for custom attrs, methods, has_MY_object_definition, get_MY_object_definition, ...

Multiple

• Ability to hook into objects' field creation
• You can use multiple TypeExtension out of the box
• Ability to add extra info via metadata? field on StrawberryObjectDefinition
• Strawberry internals are less exposed
• Non-official (current approach) ability to add custom attrs, methods etc. via setattr e.g. __strawberry_django_definition__

You could create TypeExtension that would act as Multiple TypeExtension, but not vice versa. I think that Multiple kinda clashes with and takes responsibility from strawberry.field(extensions=[...] and FieldExtension concept.