Unable to annotate union names with generics
Opened this issue · 7 comments
Describe the Bug
Strawberry's automatic type-naming for generics is ignoring annotated types on unions. Here are a few examples of what I've tried:
This causes the union type to be named SomeTypeNotFoundError:
from typing import Annotated, Generic, TypeVar, Union
import strawberry
from my_types import OtherType, SomeType
@strawberry.type
class NotFoundError:
id: strawberry.ID
message: str
T = TypeVar('T')
@strawberry.type
class ObjectQueries(Generic[T]):
...
@strawberry.field
def by_id(self, id: strawberry.ID) -> Annotated[Union[T, NotFoundError], strawberry.union("ByIdResult")]:
...
...
@strawberry.type
class Query:
@strawberry.field
def some_type_queries(self, id: strawberry.ID) -> ObjectQueries[SomeType]:
return ObjectQueries(SomeType)
@strawberry.field
def other_type_queries(self, id: strawberry.ID) -> ObjectQueries[OtherType]:
return ObjectQueries(OtherType)
schema = strawberry.Schema(Query)This causes TypeError: SomeTypeObjectQueries fields cannot be resolved.:
from typing import Annotated, Generic, TypeVar, Union
import strawberry
from my_types import OtherType, SomeType
@strawberry.type
class NotFoundError:
id: strawberry.ID
message: str
T = TypeVar('T')
@strawberry.type
class ObjectQueries(Generic[T]):
...
@strawberry.field
# NOTE annotaion moved to just the union
def by_id(self, id: strawberry.ID) -> Union[T, Annotated[NotFoundError, strawberry.union("ByIdResult")]]:
...
...
@strawberry.type
class Query:
@strawberry.field
def some_type_queries(self, id: strawberry.ID) -> ObjectQueries[SomeType]:
return ObjectQueries(SomeType)
@strawberry.field
def other_type_queries(self, id: strawberry.ID) -> ObjectQueries[OtherType]:
return ObjectQueries(OtherType)
schema = strawberry.Schema(Query)Pulling the definition up has the same behaviour in both cases:
from typing import Annotated, Generic, TypeVar, Union
import strawberry
from my_types import OtherType, SomeType
@strawberry.type
class NotFoundError:
id: strawberry.ID
message: str
T = TypeVar('T')
ByIdResult = Annotated[Union[T, NotFoundError], strawberry.union("ByIdResult", (T, NotFoundError))]
# NOTE this does the same thing, but with a linter warning:
# ByIdResult = strawberry.union("ByIdResult", (T, NotFoundError))
@strawberry.type
class ObjectQueries(Generic[T]):
...
@strawberry.field
# NOTE annotaion moved to just the union
def by_id(self, id: strawberry.ID) -> ByIdResult:
...
...
@strawberry.type
class Query:
@strawberry.field
def some_type_queries(self, id: strawberry.ID) -> ObjectQueries[SomeType]:
return ObjectQueries(SomeType)
@strawberry.field
def other_type_queries(self, id: strawberry.ID) -> ObjectQueries[OtherType]:
return ObjectQueries(OtherType)
schema = strawberry.Schema(Query)Specifying T in the union doesn't help
from typing import Annotated, Generic, TypeVar, Union
import strawberry
from my_types import OtherType, SomeType
@strawberry.type
class NotFoundError:
id: strawberry.ID
message: str
T = TypeVar('T')
ByIdResult = Union[T, Annotated[NotFoundError, strawberry.union("ByIdResult")]]
@strawberry.type
class ObjectQueries(Generic[T]):
...
@strawberry.field
# NOTE annotaion moved to just the union
def by_id(self, id: strawberry.ID) -> ByIdResult[T]:
...
...
@strawberry.type
class Query:
@strawberry.field
def some_type_queries(self, id: strawberry.ID) -> ObjectQueries[SomeType]:
return ObjectQueries(SomeType)
@strawberry.field
def other_type_queries(self, id: strawberry.ID) -> ObjectQueries[OtherType]:
return ObjectQueries(OtherType)
schema = strawberry.Schema(Query)Splitting out the error union doesn't help, this also can't resolve fields:
from typing import Annotated, Generic, TypeVar, Union
import strawberry
from my_types import OtherType, SomeType
@strawberry.type
class NotFoundError:
id: strawberry.ID
message: str
T = TypeVar('T')
ByIdError = Annotated[NotFoundError, strawberry.union("ByIdError")]
@strawberry.type
class ObjectQueries(Generic[T]):
...
@strawberry.field
# NOTE annotaion moved to just the union
def by_id(self, id: strawberry.ID) -> T | ByIdError:
...
...
@strawberry.type
class Query:
@strawberry.field
def some_type_queries(self, id: strawberry.ID) -> ObjectQueries[SomeType]:
return ObjectQueries(SomeType)
@strawberry.field
def other_type_queries(self, id: strawberry.ID) -> ObjectQueries[OtherType]:
return ObjectQueries(OtherType)
schema = strawberry.Schema(Query)System Information
- Operating system: macOS Ventura 13.2.1
- Strawberry version (if applicable): 0.218.0
Additional Context
It appears that the issue with the ones that raise an exception rather than simply having the wrong names have their problem stemming from not being able to from a union with an instance of strawberry.union
Other potentially related issues
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.
From what I can tell Union[T, Annotated[EntityNotFoundError, strawberry.union("ByIdResponse")]] "should" be the correct format for this. However, when compiling to GraphQL, Annotated[EntityNotFoundError, strawberry.union("ByIdResponse")] is first compiled to a GraphQLUnionType before attempting to create an invalid Type | Union union, which is invalid.
Union merging functionality such as with #711 would help. Does anyone know a workaround for this? Either via union merging or naming unions that contain a TypeVar?
Made a quick-and-dirty patch that merges unions (at the schema_converter level) and it worked exactly as I expected. The patch also caused 70 test failures, so I didn't exactly fix it 😅 but yes, union merging would fix this.
@jacobmoshipco is this still valid? would you like to share the patch and see if we can fix the tests? 😊
@patrick91 I'll have to check if I still have it on my home computer; I'll get back to you on that. I think it caused some other issues with unions, though.
(just switching accounts, I'm the same person as @jacobmoshipco)
/strawberry/schema/schema_converter.py, GraphQLCoreConverter.from_union
859c859
< assert isinstance(graphql_type, GraphQLObjectType)
---
> assert isinstance(graphql_type, GraphQLObjectType | GraphQLUnionType)
861c861,864
< graphql_types.append(graphql_type)
---
> if isinstance(graphql_type, GraphQLUnionType):
> graphql_types += list(graphql_type.types)
> else:
> graphql_types.append(graphql_type)Tests:
Without patch: 69 failed, 2727 passed, 160 skipped, 9 xfailed, 13 xpassed, 82 warnings in 356.93s (0:05:56)
With patch: 69 failed, 2727 passed, 160 skipped, 12 xfailed, 10 xpassed, 82 warnings in 431.60s (0:07:11)
Tbh, I may have just forgotten to do a proper control test when testing it before. Unless you (@patrick91) think this is the wrong way to patch this, or that it would cause too much of a breaking change (it does rename some unions), I can go ahead and submit it as a PR.