strawberry.Parent and type checkers / type safety

[lirsacc-mns]

Version 0.208.0 introduced the strawberry.Parent annotation; however I am a bit confused with the expectations w.r.t. type safety so opening this to write down what I think is confusing and possibly discuss improvements.

Taking the example in the docs as of today:

from dataclasses import dataclass

import strawberry


@dataclass
class UserRow:
    id_: str


@strawberry.type
class User:
    @strawberry.field
    @staticmethod
    async def name(parent: strawberry.Parent[UserRow]) -> str:
        return f"User Number {parent.id_}"


@strawberry.type
class Query:
    @strawberry.field
    def user(self) -> User:
        return UserRow(id_="1234")

Using the latest versions of strawberry and mypy (configured with the plugin) this doesn't type check:

$ mypy strawberry-parent.py 
strawberry-parent.py:23: error: Incompatible return value type (got "UserRow", expected "User")  [return-value]
Found 1 error in 1 file (checked 1 source file)

The comment in the docs act as a warning and roughly say "the type don't matter in practice"; however I am not sure it's clear enough that using strawberry.Parent as documented means giving up on the type checker for these fields.

Is the intention that users disable type checking for these cases (e.g. with a # type: ignore[return-value] comment)? Or am I missing something in the API that would make this typecheck?

---

I've started thinking about this a bit more and after reading the original discussion and PR I think a possible issue with the solution (as exposed in the docs) is that it conflates 2 concepts into one by expressing themn using a single piece of information.:

• The return type of the resolver function
• The type of the field in the schema

With that difference in mind, there's already a way to express these 2 things separately which actually works at runtime and doesn't change the API:

@strawberry.type
class Query:
    @strawberry.field(graphql_type=User)
    def user(self) -> UserRow:
        return UserRow(id_="1234")

However it's not fully type safe as this also works (until it breaks at query execution time):

@strawberry.type
class Query:
    @strawberry.field(graphql_type=User)
    def user(self) -> int:
        return 1234

One approach could be to have strawberry.field validate that the return value is consistent with the strawberry.Parent? However that leads to a second issue:

@dataclass
class UserRow:
    id_: str


@dataclass
class NotAUserRow:
    name: str


@strawberry.type
class User:
    @strawberry.field
    @staticmethod
    async def name(parent: strawberry.Parent[UserRow]) -> str:
        return f"User Number {parent.id_}"

    @strawberry.field
    @staticmethod
    async def name2(parent: strawberry.Parent[NotAUserRow]) -> str:
        return f"User Number {parent.name}"

Is technically allowed and works (until it breaks at query execution time) although I assume is not the intended usage. Equally that's solvable by validating in strawberry.type that all parent annotations are consistent.

So I reckon having these 2 checks (whether at schema definition time, in the mypy plugin, or both):

• strawberry.Parent usage must be consistent across a single type
• A field's return type must be assignable to the graphql_type's parent type

would allow maintaining the same API but in a way which type-checks properly while preserving the API and not relying on type: ignore.

If there's agreement it's worth doping I'd be happy to take it on if it helps.

[patrick91]

Hi there! thanks for the great write up! 😊

In my head this was the recommended option:

@strawberry.type
class Query:
    @strawberry.field(graphql_type=User)
    def user(self) -> UserRow:
        return UserRow(id_="1234")

but yes it is not type same unfortunately.

I was thinking that maybe we could add support for protocols (if they don't work already), which could help a bit with type safety, but it really depends on what you're doing 😊

Regarding your suggestion:

@dataclass
class UserRow:
    id_: str


@dataclass
class NotAUserRow:
    name: str


@strawberry.type
class User:
    @strawberry.field
    @staticmethod
    async def name(parent: strawberry.Parent[UserRow]) -> str:
        return f"User Number {parent.id_}"

    @strawberry.field
    @staticmethod
    async def name2(parent: strawberry.Parent[NotAUserRow]) -> str:
        return f"User Number {parent.name}"

this would thrown an error/warning on name2, right?

[lirsacc-mns]

Thanks for the quick reply!

I was thinking that maybe we could add support for protocols (if they don't work already), which could help a bit with type safety, but it really depends on what you're doing 😊

Unless I am missing something, I am not entirely sure this solves the issue. Would the protocol be the parent type or the GraphQL type?

From that example If User is turned into a protocol, e.g.

@strawberry.type
class User(Protocol):
    id_: str

    @strawberry.field
    @staticmethod
    def name(parent: strawberry.Parent[User]) -> str:
        return f"User Number {parent.id_}"

then UserRow still isn't assignable to User (missing name). Same if we add a protocol and make that the strawberry.Parent; UserRow would be assignable to it but you'd still have the mismatch at the field level:

class UserProto(Protocol):
    id_: str


@dataclass
class UserRow:
    id_: str


@strawberry.type
class User(Protocol):
    id_: str

    @strawberry.field
    @staticmethod
    def name(parent: strawberry.Parent[UserProto]) -> str:
        return f"User Number {parent.id_}"


@strawberry.type
class Query:
    # UserProto isn't in play here.
    @strawberry.field
    def user(self) -> User:
        return UserRow(id_="1234")

    # Or:

    @strawberry.field(graphql_type=User)
    def user(self) -> UserProto:
        return UserRow(id_="1234")

Regarding your suggestion:

...

this would thrown an error/warning on name2, right?

Yes this breaks but at runtime but only at query execution time (AttributeError when resolving name2 because UserRow doesn't have name). Although I reckon that's less of an issue as it's fairly obviously a missuse of the API. Equally if I were to return an int instead of UserRow we get runtime errors on query execution.

To clarify: I think the main point for me is that if I need to rely on strawberry.Parent then I have to give up type safety / schema level safety for the and have to rely on query-execution errors. Hence the rough idea to introduce consistency checks and warnings at schema-build time to get back some of the safety (doing it in the mypy plugin might be possible but I am not sold that it's the right place).

With the protocol as well I think the checking would still allow them so it should be possible to use a protocol for the strawberry.Parent so resolver can return different implementations of the protocol as they'd all be assignable.

[patrick91]

oh I think I see what you mean, the issue is that the query field might return something that's not a User, right?

So you'd suggest us to check that the query fields are returning the same type that's defined as the parent field in resolvers?

@dataclass
class UserRow:
    id_: str


@strawberry.type
class User(Protocol):
    id_: str

    @strawberry.field
    @staticmethod
    def name(parent: strawberry.Parent[User]) -> str:
        return f"User Number {parent.id_}"


@strawberry.type
class Query:
    @strawberry.field(graphql_type=User)
    def user(self) -> UserProto:  # we'd fail here because we aren't returning the right thing?
        return UserRow(id_="1234")

[lirsacc-mns]

Yep that's it.

To spell it out as precisely as I can:

Validate that the return type of a resolver (UserRow in that example) is the same or a subclass of the strawberry.Parent[...] type used on fields of the graphql_type type (User here).

This assumes that given a schema type (here User) usage of strawberry.Parent is consistent across all fields; which I assume is the intent anyway. If it's not, we could replace "is the same (or maybe a subclass)" by "is assignable" (although less clear how this can be checked).

[patrick91]

I guess we could have a strict mode that runs at schema creation time 😊

Or maybe a plugin for Mypy or potentially even better a linter

What do you think?