-
Notifications
You must be signed in to change notification settings - Fork 47
Add type hints using basedpyright #85
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7aed8ec
ca44a06
f5cfca0
dd65e9f
c26d2bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,3 +41,12 @@ jobs: | |
| - uses: astral-sh/ruff-action@v3 | ||
| - run: ruff check | ||
| - run: ruff format --check | ||
| mypy: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: astral-sh/[email protected] | ||
| with: | ||
| python-version: '3.12' | ||
| - run: uvx poetry install --extras=ssr | ||
| - run: uvx poetry run mypy . | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,11 @@ | ||
| def deep_transform_callables(prop): | ||
| from typing import Any | ||
|
|
||
|
|
||
| def deep_transform_callables(prop: Any) -> Any: | ||
| if not isinstance(prop, dict): | ||
| return prop() if callable(prop) else prop | ||
|
|
||
| for key in list(prop.keys()): | ||
| prop[key] = deep_transform_callables(prop[key]) | ||
|
|
||
| return prop | ||
|
|
||
|
|
||
| def validate_type(value, name, expected_type): | ||
| if not isinstance(value, expected_type): | ||
| raise TypeError( | ||
| f"Expected {expected_type.__name__} for {name}, got {type(value).__name__}" | ||
| ) | ||
|
|
||
| return value |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,12 +1,13 @@ | ||||||||
| from functools import wraps | ||||||||
| from http import HTTPStatus | ||||||||
| from json import dumps as json_encode | ||||||||
| from typing import Any, Callable | ||||||||
|
|
||||||||
| from django.core.exceptions import ImproperlyConfigured | ||||||||
| from django.http import HttpRequest, HttpResponse | ||||||||
| from django.template.loader import render_to_string | ||||||||
|
|
||||||||
| from .helpers import deep_transform_callables, validate_type | ||||||||
| from .helpers import deep_transform_callables | ||||||||
| from .prop_classes import DeferredProp, IgnoreOnFirstLoadProp, MergeableProp | ||||||||
| from .settings import settings | ||||||||
|
|
||||||||
|
|
@@ -15,7 +16,7 @@ | |||||||
| # a mock module | ||||||||
| import requests | ||||||||
| except ImportError: | ||||||||
| requests = None | ||||||||
| requests = None # type: ignore[assignment] | ||||||||
|
|
||||||||
|
|
||||||||
| INERTIA_REQUEST_ENCRYPT_HISTORY = "_inertia_encrypt_history" | ||||||||
|
|
@@ -26,51 +27,55 @@ | |||||||
|
|
||||||||
|
|
||||||||
| class InertiaRequest(HttpRequest): | ||||||||
| def __init__(self, request): | ||||||||
| def __init__(self, request: HttpRequest): | ||||||||
| super().__init__() | ||||||||
| self.__dict__.update(request.__dict__) | ||||||||
|
|
||||||||
| @property | ||||||||
| def inertia(self): | ||||||||
| def inertia(self) -> dict[str, Any]: | ||||||||
| inertia_attr = self.__dict__.get("inertia") | ||||||||
| return ( | ||||||||
| inertia_attr.all() if inertia_attr and hasattr(inertia_attr, "all") else {} | ||||||||
| ) | ||||||||
|
|
||||||||
| def is_a_partial_render(self, component): | ||||||||
| def is_a_partial_render(self, component: str) -> bool: | ||||||||
| return ( | ||||||||
| "X-Inertia-Partial-Data" in self.headers | ||||||||
| and self.headers.get("X-Inertia-Partial-Component", "") == component | ||||||||
| ) | ||||||||
|
|
||||||||
| def partial_keys(self): | ||||||||
| def partial_keys(self) -> list[str]: | ||||||||
| return self.headers.get("X-Inertia-Partial-Data", "").split(",") | ||||||||
|
|
||||||||
| def reset_keys(self): | ||||||||
| def reset_keys(self) -> list[str]: | ||||||||
| return self.headers.get("X-Inertia-Reset", "").split(",") | ||||||||
|
|
||||||||
| def is_inertia(self): | ||||||||
| def is_inertia(self) -> bool: | ||||||||
| return "X-Inertia" in self.headers | ||||||||
|
|
||||||||
| def should_encrypt_history(self): | ||||||||
| return validate_type( | ||||||||
| getattr( | ||||||||
| self, | ||||||||
| INERTIA_REQUEST_ENCRYPT_HISTORY, | ||||||||
| settings.INERTIA_ENCRYPT_HISTORY, | ||||||||
| ), | ||||||||
| expected_type=bool, | ||||||||
| name="encrypt_history", | ||||||||
| def should_encrypt_history(self) -> bool: | ||||||||
| should_encrypt = getattr( | ||||||||
| self, INERTIA_REQUEST_ENCRYPT_HISTORY, settings.INERTIA_ENCRYPT_HISTORY | ||||||||
| ) | ||||||||
| if not isinstance(should_encrypt, bool): | ||||||||
| raise TypeError( | ||||||||
| f"Expected bool for encrypt_history, got {type(should_encrypt).__name__}" | ||||||||
| ) | ||||||||
| return should_encrypt | ||||||||
|
|
||||||||
|
|
||||||||
| class BaseInertiaResponseMixin: | ||||||||
| def page_data(self): | ||||||||
| clear_history = validate_type( | ||||||||
| self.request.session.pop(INERTIA_SESSION_CLEAR_HISTORY, False), | ||||||||
| expected_type=bool, | ||||||||
| name="clear_history", | ||||||||
| ) | ||||||||
| request: InertiaRequest | ||||||||
| component: str | ||||||||
| props: dict[str, Any] | ||||||||
| template_data: dict[str, Any] | ||||||||
|
|
||||||||
| def page_data(self) -> dict[str, Any]: | ||||||||
| clear_history = self.request.session.pop(INERTIA_SESSION_CLEAR_HISTORY, False) | ||||||||
| if not isinstance(clear_history, bool): | ||||||||
| raise TypeError( | ||||||||
| f"Expected bool for clear_history, got {type(clear_history).__name__}" | ||||||||
| ) | ||||||||
|
|
||||||||
| _page = { | ||||||||
| "component": self.component, | ||||||||
|
|
@@ -91,7 +96,7 @@ def page_data(self): | |||||||
|
|
||||||||
| return _page | ||||||||
|
|
||||||||
| def build_props(self): | ||||||||
| def build_props(self) -> Any: | ||||||||
| _props = { | ||||||||
| **(self.request.inertia), | ||||||||
| **self.props, | ||||||||
|
|
@@ -107,18 +112,18 @@ def build_props(self): | |||||||
|
|
||||||||
| return deep_transform_callables(_props) | ||||||||
|
|
||||||||
| def build_deferred_props(self): | ||||||||
| def build_deferred_props(self) -> dict[str, Any] | None: | ||||||||
| if self.request.is_a_partial_render(self.component): | ||||||||
| return None | ||||||||
|
|
||||||||
| _deferred_props = {} | ||||||||
| _deferred_props: dict[str, Any] = {} | ||||||||
| for key, prop in self.props.items(): | ||||||||
| if isinstance(prop, DeferredProp): | ||||||||
| _deferred_props.setdefault(prop.group, []).append(key) | ||||||||
|
|
||||||||
| return _deferred_props | ||||||||
|
|
||||||||
| def build_merge_props(self): | ||||||||
| def build_merge_props(self) -> list[str]: | ||||||||
| return [ | ||||||||
| key | ||||||||
| for key, prop in self.props.items() | ||||||||
|
|
@@ -129,7 +134,7 @@ def build_merge_props(self): | |||||||
| ) | ||||||||
| ] | ||||||||
|
|
||||||||
| def build_first_load(self, data): | ||||||||
| def build_first_load(self, data: Any) -> str: | ||||||||
| context, template = self.build_first_load_context_and_template(data) | ||||||||
|
|
||||||||
| try: | ||||||||
|
|
@@ -151,7 +156,9 @@ def build_first_load(self, data): | |||||||
| using=None, | ||||||||
| ) | ||||||||
|
|
||||||||
| def build_first_load_context_and_template(self, data): | ||||||||
| def build_first_load_context_and_template( | ||||||||
| self, data: Any | ||||||||
| ) -> tuple[dict[str, Any], str]: | ||||||||
| if settings.INERTIA_SSR_ENABLED: | ||||||||
| try: | ||||||||
| response = requests.post( | ||||||||
|
|
@@ -178,14 +185,14 @@ class InertiaResponse(BaseInertiaResponseMixin, HttpResponse): | |||||||
|
|
||||||||
| def __init__( | ||||||||
| self, | ||||||||
| request, | ||||||||
| component, | ||||||||
| props=None, | ||||||||
| template_data=None, | ||||||||
| headers=None, | ||||||||
| *args, | ||||||||
| **kwargs, | ||||||||
| ): | ||||||||
| request: HttpRequest, | ||||||||
| component: str, | ||||||||
| props: dict[str, Any] | None = None, | ||||||||
| template_data: dict[str, Any] | None = None, | ||||||||
| headers: dict[str, Any] | None = None, | ||||||||
| *args: Any, | ||||||||
| **kwargs: Any, | ||||||||
| ) -> None: | ||||||||
| self.request = InertiaRequest(request) | ||||||||
| self.component = component | ||||||||
| self.props = props or {} | ||||||||
|
|
@@ -208,19 +215,30 @@ def __init__( | |||||||
| else: | ||||||||
| content = self.build_first_load(data) | ||||||||
|
|
||||||||
| super().__init__( | ||||||||
| *args, | ||||||||
| content=content, | ||||||||
| headers=_headers, | ||||||||
| **kwargs, | ||||||||
| ) | ||||||||
| if args: | ||||||||
| super().__init__( | ||||||||
| *args, | ||||||||
|
||||||||
| *args, | |
| *args, | |
| content=content, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrgalopes sorry about the delay here! Could you comment on why this change is necessary? Other than this question I think we're good to merge!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Brandon! Sorry for taking so long to respond as well. About the change, leaving it like this:
super().__init__(
*args,
content=content,
headers=_headers,
**kwargs,
)Got me the following error from mypy: inertia/http.py:230: error: "__init__" of "HttpResponse" gets multiple values for keyword argument "content" [misc]
That is because HttpResponse has a constructor that is like this:
def __init__(self, content=b"", *args, **kwargs):
...From what I could understand, mypy can't know if content is a positional argument coming from *args or if it comes from the variable defined in the function body. I made it into an if statement so that the client can decide if they want to override the content using *args.
On second thought, this works for the default case and passes mypy, but fails if the caller passes content as a keyword argument (not sure if that's something we'd like to support either way):
super().__init__(
content,
*args,
headers=_headers,
**kwargs,
)What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh I see. I always get hung up on these "only matters for the linter" type of situations 😄 I think this makes sense though, thanks for explaning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lazy annotations and
if TYPE_CHECKINGmight be a nice way to avoid unessecary imports.