Add *args to Middleware and improve its type hints#2381
Add *args to Middleware and improve its type hints#2381adriangb merged 4 commits intoKludex:masterfrom
*args to Middleware and improve its type hints#2381Conversation
Use ParamSpec to provide concrete type annotations for middleware's parameters.
pawelrubin
force-pushed
the
feat/improve-type-annotations-for-middleware
branch
from
0f4c8c5 to
162440c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
pawelrubin
force-pushed
the
feat/improve-type-annotations-for-middleware
branch
from
3819c8e to
d22a612
Compare
pawelrubin
force-pushed
the
feat/improve-type-annotations-for-middleware
branch
from
69bee8d to
0d36db7
Compare
Kludex
changed the title
*args to Middleware and its type hints
Kludex
changed the title
*args to Middleware and its type hints*args to Middleware and improve its type hints
starlette/applications.py
Outdated
| def add_middleware(self, middleware_class: type, **options: typing.Any) -> None: | ||
| def add_middleware( | ||
| self, | ||
| middleware_class: typing.Type[MiddlewareClass[P]], |
There was a problem hiding this comment.
I figured that we could achieve more precise annotation via typing.Protocol. Moreover, this ensures that the middleware instance does adhere to the ASGI protocol.
adriangb
left a comment
adriangb
left a comment
There was a problem hiding this comment.
Looks good to me other than a slight preference for not adding new public symbols
starlette/middleware/__init__.py
Outdated
| P = ParamSpec("P") | ||
|
|
||
|
|
||
| class MiddlewareClass(Protocol[P]): # pragma: no cover |
There was a problem hiding this comment.
Can we somehow make this private?
There was a problem hiding this comment.
Also why is there pragma here? Doesn't it get run just by being imported?
There was a problem hiding this comment.
This particular line - as well as the class body - does get run when imported. However, the empty body (...) of __init__ and __call__ does not, since those are not being called. Putting pragma at the top of the class was merely a shortcut - not to put pragma in both of those functions.
Though it's probably best to be explicit. Changed.
There was a problem hiding this comment.
Makes total sense! It just confused me.
pawelrubin
force-pushed
the
feat/improve-type-annotations-for-middleware
branch
from
7b26974 to
5680abb
Compare
pawelrubin
force-pushed
the
feat/improve-type-annotations-for-middleware
branch
from
5680abb to
2d7eb8c
Compare
3 participants
Use ParamSpec to provide concrete type annotations for middleware's parameters.
Summary
See #2380
Checklist
I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.I've updated the documentation accordingly.