strawberry-graphql/strawberry

Can Strawberry created dataclasses be `frozen` by default?

Opened this issue · 3 comments

kkom commented

Feature Request Type

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

Need

Since 1.1.328 Pyright type checker has become more strict w.r.t. to detecting incompatible variable overrides.

Specifically code like this:

class MyParent:
  my_field: int | str

class MyChild(MyParent):
  my_field: int  # `reportIncompatibleVariableOverride` detected here

Now triggers a type error, because of this failure scenario:

def mutate_parent(p: MyParent) -> None:
  p.my_field = "foo"

def process_child(c: MyChild) -> None:
  mutate_parent(c)

  c.my_field  # the typechecker believes the type is `int`, but the actual value at runtime is `"foo"` and the type is `str`

However, if MyParent was a frozen dataclass everything would be fine – because Pyright would be convinced that the value isn't mutated when processing the value using the wider parent type.

A scenario where this is useful is interaction between GraphQL types and interfaces they implement. It's common for a GraphQL interface to be wider than the specific type that implements this. In order to represent this through Python inheritance, any such fields needs to be implemented as a Python method, rather than instance/class variables. This is because the return types of the former are interpreted as immutable by Pyright.

Therefore, I suggest that dataclasses created to represent Strawberry GraphQL types are frozen dataclasses. I don't think there is any real use case for mutating field values in an already constructed Python object.

Implementation

I'm sure the solution is somewhere in PEP 681, especially because there are plenty of occurrences of the term frozen in its text. However, it's not immediately clear to me how this should be done – I've never written a dataclass transform myself.

Happy to dig into it a little bit myself, if nobody else has an immediate idea how to do it though. :)

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
kkom commented

Okay, I think we need to add frozen_default=True in places like this:

order_default=True, kw_only_default=True, field_specifiers=(field, StrawberryField)

order_default=True, kw_only_default=True, field_specifiers=(field, StrawberryField)

See: python/cpython#99957 and the updated spec here: https://typing.readthedocs.io/en/latest/spec/dataclasses.html#dataclass-transform-parameters

kkom commented

I've created #3397 for it – I suggest we continue the discussion there, as it's closer to code :)

kkom commented

One step closer to drawing the issue I created to an unfortunately unsatisfying end. The performance hit of this is pretty substantial, see more: #3397 (comment)