Add support for implicit UNSET as a type annotation

[sarahhenkens]

Currently, we need to set the default value to strawberry.arguments.UNSET to support optional arguments. This object has the Any annotation which hides itself from type checkers.

Example:

id: Optional[strawberry.ID] = UNSET

MyPy will consider id to be a Union[ID, None] while in reality, it's a Union[ID, None, _Unset]. This results in potential bugs in code where the type-checker would catch it.

For example, the following snippet will raise an error without a type checker catching it:

def foo(id: Optional[str] = UNSET) -> None:
  if id is not None:
    print(id.lower()) # Will attempt to call `lower()` on `UNSET`

Proposal

My proposal is to introduce a new type alias as a first-class citizen in the Strawberry type system:

T = TypeVar("T")
UnsetOrOptional = Union[_Unset, Optional[T]]

And its usage in resolvers:

@strawberry.field
def foo(id: UnsetOrOptional[strawberry.ID]) -> None:
  pass

• The default assignment of the arguments remain used for actual default values in GraphQL
• The body of our resolver has the current triple Union to enforce it to check for unset as well
• The upstream logic will automatically assign UNSET to its value if it was not included in the request

Just optional

This allows us to also provide the option for resolvers to define a simpler default behavior when we only care about strawberry.ID vs None:

@strawberry.field
def foo(id: Optional[strawberry.ID]) -> None:
  pass

• No default assignment needed if you don't want it to show up in the generated GraphQL schema
• The upstream logic will automatically assign None to its value if it was not included in the request

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.

[jkimbo]

I wonder if it's simpler to just make Union[Optional[str], UNSET] work rather than introduce a new type? What do you think @sarahhenkens ? My thinking is that even with a new type using a Union should also work so why not just make that the way to do it.

Note: Union[str, None, UNSET] should also work.

[sarahhenkens]

I think it could, it does require you to explicitly set the default value to UNSET though

[jkimbo]

Also what should happen if you define Union[str, UNSET]. Is that a valid state?

[joeydebreuk]

I think it could, it does require you to explicitly set the default value to UNSET though

I think that should be pretty easy to solve with a custom scalar?
https://strawberry.rocks/docs/types/scalars

[sarahhenkens]

From a typing perspective, that is a valid value but it's impossible to reach there since this should be emitted into a String!.

When using the union method, I currently get the following error:

strawberry.exceptions.InvalidUnionType: Union type `_Unset` is not a Strawberry type

[patrick91]

@sarahhenkens we (well @jkimbo :D) are going to work on this as discussed in our monthly meeting 😊 #986

[sarahhenkens]

A minor fix to the annotation parser to allow for stricter type annotation for optional and unsettable field args #1467

[lukaspiatkowski]

This is a major flaw in the libary IMO. My current workaround is to use strawberry.unset.UnsetType directly, like so:

from strawberry.unset import UnsetType
def foo(id: str | None | UnsetType = UnsetType()) -> None:
  if id is not None or isinstance(id, UnsetType):
    print(id.lower())

The type checker understands that if you provide = UnsetType() then the type annotation has to include UnsetType and that kicks in the proper type protection in the body of the function.