Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the starlette/middleware/__init__.py file to remove the dependency on starlette.types.ASGIApp and replace it with a local, more permissive type alias _ASGIApp. This change aims to prevent type errors when using middleware with type checking.
Key Changes:
- Introduced local type aliases (
_Scope,_Receive,_Send,_ASGIApp) usingAnyfor more permissive typing - Removed the import of
ASGIAppfromstarlette.types - Updated
_MiddlewareFactoryprotocol to use the new_ASGIApptype - Reformatted the
Middleware.__init__method to a single line
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
starlette/middleware/__init__.py
Outdated
| _Receive = Callable[[], Awaitable[Any]] | ||
| _Send = Callable[[Any], Awaitable[None]] | ||
| _ASGIApp = Callable[[_Scope, _Receive, _Send], Awaitable[None]] | ||
| """A more permissive version of ASGIApp that doesn't cause type errors.""" |
There was a problem hiding this comment.
The docstring for _ASGIApp should clarify when and why this permissive version should be used instead of the standard ASGIApp from starlette.types. Consider expanding it to: 'A more permissive version of ASGIApp used in middleware protocols to avoid type errors when middleware implementations use more specific ASGI types.'
| """A more permissive version of ASGIApp that doesn't cause type errors.""" | |
| """ | |
| A more permissive version of ASGIApp used in middleware protocols to avoid type errors | |
| when middleware implementations use more specific ASGI types. This type alias should be | |
| used instead of the standard ASGIApp from starlette.types in cases where middleware | |
| needs to accept a broader range of ASGI applications without strict type constraints. | |
| """ |
|
|
||
| class _MiddlewareFactory(Protocol[P]): | ||
| def __call__(self, app: ASGIApp, /, *args: P.args, **kwargs: P.kwargs) -> ASGIApp: ... # pragma: no cover | ||
| def __call__(self, app: _ASGIApp, /, *args: P.args, **kwargs: P.kwargs) -> _ASGIApp: ... # pragma: no cover |
There was a problem hiding this comment.
This statement has no effect.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| _Scope = Any | ||
| _Receive = Callable[[], Awaitable[Any]] |
There was a problem hiding this comment.
These internal type aliases lack documentation explaining their purpose. Consider adding a comment explaining why Any is used for _Scope instead of the more specific MutableMapping[str, Any] from starlette.types.Scope, and why _Receive returns Any instead of Message. This would help maintainers understand the intentional loosening of types.
| _Scope = Any | |
| _Receive = Callable[[], Awaitable[Any]] | |
| # Internal type alias for ASGI scope. | |
| # We use `Any` here instead of `MutableMapping[str, Any]` (from `starlette.types.Scope`) | |
| # to allow for compatibility with different ASGI implementations and to avoid type errors | |
| # when interacting with libraries that may not strictly adhere to the more specific type. | |
| _Scope = Any | |
| # Internal type alias for ASGI receive callable. | |
| # The return type is `Any` instead of `Message` to permit broader compatibility with | |
| # various ASGI implementations, some of which may not use the exact `Message` type. | |
| _Receive = Callable[[], Awaitable[Any]] | |
| # Internal type alias for ASGI send callable. | |
| # The argument is typed as `Any` instead of `Message` for the same reason as above: | |
| # to allow for flexibility and compatibility with different ASGI implementations. |
|
|
||
| class _MiddlewareFactory(Protocol[P]): | ||
| def __call__(self, app: ASGIApp, /, *args: P.args, **kwargs: P.kwargs) -> ASGIApp: ... # pragma: no cover | ||
| def __call__(self, app: _ASGIApp, /, *args: P.args, **kwargs: P.kwargs) -> _ASGIApp: ... # pragma: no cover |
There was a problem hiding this comment.
This statement has no effect.
Uh oh!
There was an error while loading. Please reload this page.