Use of asgiref typing

[Kludex]

Checklist

• There are no similar issues or pull requests for this yet.
• @JayH5 and @Kludex are positive about this idea, but it was not discussed on our Gitter channel.

Is your feature related to a problem? Please describe.

On uvicorn we started a mypy compliance goal since some time ago, and we're about to accomplish it. On this road, we decided to use asgiref typing to annotate ASGI related types.

What I propose is that we do the same for Starlette.

Describe the solution you would like.

Remove the types.py module, and annotate everything that is needed with asgiref.typing types.

There's no matching type to Message, as discussed on #1206 . So even if it would be more pleasant to our users, unfortunately it will be more troublesome to follow that path.

Describe alternatives you considered

Use Message as Union[ASGISendCallable, ASGIReceiveCallable] and solve the mypy issues. It will be more pleasant to ours users.

[Kludex]

I guess the first step here would be to replace the Message type by two different message types, one for receive and another one for send.

[aminalaee]

@Kludex if we can maybe split this into multiple part, I can help with it too.

[Kludex]

The problem is that I'm not 100% sure, but I think what makes the issue hard to solve is the Message type. You'll see the mypy errors if you experiment some changes locally.

That being said... I believe the approach would be:

1. Split Message into ASGISendEvent and ASGIReceiveEvent - naming matching asgiref.
2. Rename the other types to asgiref (just renaming, not introduce the package yet).
3. Remove the types definition and import from asgiref - this should be straightforward based on step 2.

[br3ndonland]

The problem is that I'm not 100% sure, but I think what makes the issue hard to solve is the Message type. You'll see the mypy errors if you experiment some changes locally.

That being said... I believe the approach would be:

1. Split Message into ASGISendEvent and ASGIReceiveEvent - naming matching asgiref.
2. Rename the other types to asgiref (just renaming, not introduce the package yet).
3. Remove the types definition and import from asgiref - this should be straightforward based on step 2.

I helped with type-annotating Uvicorn, and I'm happy to help here as well.

I've already made some good progress, and I'm on step 3 now. I need to correct a few type errors, now that we're importing from the more specific asgiref types.

Should have a PR up shortly.

[Kludex]

@br3ndonland Cool! I'll be happy to review it.

Just take in consideration that the steps mentioned were also a way to minimize the diff and make the review process easier. 🙏

[br3ndonland]

Just take in consideration that the steps mentioned were also a way to minimize the diff and make the review process easier. 🙏

Totally. I know these refactorings can create large diffs. It can be a lot to review. 😫

I'm trying to keep my commits organized, so you can easily see the steps taken, and walk through commit-by-commit. We can also break it up into multiple PRs if needed.

[Kludex]

Perfect! Thanks @br3ndonland !

[Kludex]

Hmmm... Converting this issue made a pretty ugly discussion page... I wish I could revert...

Anyway, opinions about what's written here are welcome. @encode/maintainers

[relsunkaev]

I think it would be best to create a separate package ("asgi-types" maybe?) to not have to depend on all the other stuff in asgiref.

[adriangb]

I think that would only work if everyone adopted that. If we could convince asgiref to itself depend on asgi-types, that would be a win.

[tdg5]

I'm sorry to see this discussion has lost momentum. I think there's value in making these changes, otherwise I have to add type ignores to any code that pairs up starlette/FastAPI with hypercorn.

I just spent a couple of hours (not for the first time) trying to figure out where I was going wrong with making mypy believe that starlette and hypercorn can be friends.