Allow Response in route type hints #85
Conversation
WalkthroughType annotations expanded to allow decorators hx/page to accept functions returning either T or Response in both core_decorators and HTMY. No runtime logic changes. Tests receive minor formatting. Package version bumped to 3.0.1. CI workflow job renamed from “Linters” to “Tests”. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (5)
fasthx/core_decorators.py(4 hunks)fasthx/htmy.py(2 hunks)pyproject.toml(1 hunks)tests/test_core_decorators.py(2 hunks)tests/test_htmy.py(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Tests
tests/test_htmy.py
[error] 34-34: FastAPIError: Invalid args for response field! Hint: check that {type_} is a valid Pydantic field type.
🔇 Additional comments (9)
pyproject.toml (1)
3-3: Version bump looks good.The patch version increment from 3.0.0 to 3.0.1 is appropriate for type hint improvements without runtime behavior changes.
tests/test_core_decorators.py (2)
49-49: Type hint correctly reflects decorator capabilities.The
| Responseaddition aligns with the updated decorator signatures, even though this specific route doesn't return a Response object. The type hint accurately represents what the decorator allows.
58-58: Type hint correctly reflects decorator capabilities.The
| Responseaddition aligns with the updated decorator signatures, even though this specific route doesn't return a Response object. The type hint accurately represents what the decorator allows.fasthx/htmy.py (2)
141-141: Type signature correctly propagates Response support.The signature update to accept and return
T | Responseproperly reflects the decorator's capability to handle routes that may return Response objects directly, aligning with the core decorator changes.
166-166: Type signature correctly propagates Response support.The signature update to accept and return
T | Responseproperly reflects the decorator's capability to handle routes that may return Response objects directly, aligning with the core decorator changes.fasthx/core_decorators.py (4)
19-19: Type signature correctly reflects runtime behavior.The signature update to return
Callable[[MaybeAsyncFunc[P, T | Response]], Callable[P, Coroutine[None, None, T | Response]]]accurately represents that routes can return eitherTorResponse, with the existing runtime logic at line 58 already handling Response objects by returning them directly.
34-36: Inner decorator signature properly accepts Response returns.The updated signature
MaybeAsyncFunc[P, T | Response]correctly reflects that the decorated function can return a Response object, which is already handled by the runtime check at line 58.
89-89: Type signature correctly reflects runtime behavior.The signature update to return
Callable[[MaybeAsyncFunc[P, T | Response]], Callable[P, Coroutine[None, None, Response]]]accurately represents that routes can return eitherTorResponse, with the existing runtime logic at line 114 already handling Response objects by returning them directly.
99-99: Inner decorator signature properly accepts Response returns.The updated signature
MaybeAsyncFunc[P, T | Response]correctly reflects that the decorated function can return a Response object, which is already handled by the runtime check at line 114.
(locally it seems ok with same versions)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
.github/workflows/tests.yml(1 hunks)tests/test_htmy.py(1 hunks)
🔇 Additional comments (1)
.github/workflows/tests.yml (1)
13-13: LGTM! Job name now matches the workflow purpose.The rename from "Linters" to "Tests" accurately reflects what this job does and aligns with the workflow filename.
| def htmx_or_data( | ||
| response: Response, | ||
| ) -> list[User]: |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor formatting inconsistency.
The single parameter is now on its own line with a trailing comma, which differs from the style used for other single-parameter functions in this file (e.g., line 103). While the trailing comma is good practice for future-proofing, the inline style is more common for single parameters.
Note: The past review comments suggesting | Response in the return type and response_model=None in the decorator appear to be outdated and don't apply to the current code state.
🤖 Prompt for AI Agents
In tests/test_htmy.py around lines 41 to 43, the function signature for
htmx_or_data places the single parameter on its own line with a trailing comma
which is inconsistent with other single-parameter functions in this file; change
the signature to a single-line style like def htmx_or_data(response: Response)
-> list[User]: to match existing style (remove the extra line break and trailing
comma) while keeping the same parameter type and return annotation.
This is only relevant for the
htmyintegration and the core decorators, because with Jinja the type information is not relevant (alreadyAny).Summary by CodeRabbit