strawberry icon indicating copy to clipboard operation
strawberry copied to clipboard

Add custom return type annotation for subscriptions

Open jaydenwindle opened this issue 5 years ago • 4 comments

Currently it appears that when defining a subscription the return type annotation must be set to typing.AsyncGenerator[str, None], like so:

@strawberry.type
class Subscription:
    @strawberry.subscription
    async def time(self, info) -> typing.AsyncGenerator[str, None]:
        ...

Do we need to use typing.AsyncGenerator as the return type for subscription definitions? I'm not sure that I love that dev experience. Would it be bad practice to use raw types as the return type? Something like:

@strawberry.subscription
async def time(self, info) -> str:
    ...

Or maybe a we could use a custom return type for subscriptions:

@strawberry.subscription
async def time(self, info) -> strawberry.subscription_result(str):
    ...

jaydenwindle avatar Jun 24 '19 19:06 jaydenwindle

@jaydenwindle yes, that's a very good point, I don't like that too, not sure what our best options are.

The reason why I was using typing.AsyncGenerator is because MyPy was complaining :D

I'd say I like your last suggestion, but maybe it should look like this:

@strawberry.subscription
async def time(self, info) -> strawberry.SubscriptionResult[str]:
    ...

so it looks more like a python typing type ;)

We should maybe also have a look at MyPy plugins, like the one for dataclasses: https://github.com/python/mypy/blob/master/mypy/plugins/dataclasses.py

patrick91 avatar Jun 25 '19 07:06 patrick91

I like that option too!

I'll have to look a little deeper into how MyPy does its magic, but I think if we define strawberry.SubscriptionResult as a custom type that's just a wrapper around typing.AsyncGenerator it should keep MyPy from complaining.

jaydenwindle avatar Jun 25 '19 16:06 jaydenwindle

Do we still want this @patrick91? Personally I'm fine with AsyncGenerator[str, None] since I see it a lot in code using asyncio. But adding a type alias is still an option of course :)

(sorry for digging this up, I'm trying to get our issue count under control :D )

DoctorJohn avatar Aug 04 '22 11:08 DoctorJohn

I think an alias would be nice to have, at least until we get optional type params :D

patrick91 avatar Aug 04 '22 22:08 patrick91