search

litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.64k stars 382 forks source link

Bug: Using `from __future__ import annotations` breaks template support #1271

Closed Nadock closed 1 year ago

Nadock commented 1 year ago

Describe the bug If you include from __future__ import annotations in the same file as a route handler that returns a starlite.Template the template will not be rendered. Instead, the starlite.Template object is converted to JSON and that is returned instead.

To Reproduce Below is code that reproduces the error for me.

# app.py
from __future__ import annotations

import pathlib

import starlite
from starlite.contrib import jinja

@starlite.get("/")
def index() -> starlite.Template:
    return starlite.Template(name="repro.html")

app = starlite.Starlite(
    route_handlers=[index],
    template_config=starlite.TemplateConfig(
        directory=pathlib.Path("./templates"),
        engine=jinja.JinjaTemplateEngine,
    ),
)
<!-- templates/repro.html -->
<!DOCTYPE html>
<html lang="en-au">

<head>
    <meta content="text/html;charset=utf-8" http-equiv="Content-Type">
    <meta content="utf-8" http-equiv="encoding">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <meta name="color-scheme" content="dark light">
    <title>from __future__ import annotations</title>
</head>

<body>
    <h1>Hello, world!</h1>
</body>

Additional context With the from __future__ import annotations line: Screenshot 2023-03-04 at 17 41 19

Without the from __future__ import annotations line:

Screenshot 2023-03-04 at 17 46 14

I've only just started using Starlite so I'm not sure if there is something I missed in the documentation that says you shouldn't use from __future__ import annotations. But even if there is, maybe Starlite could raise an exception when you do this.

[!NOTE]
While we are open for sponsoring on GitHub Sponsors and OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.

Fund with Polar

Goldziher commented 1 year ago

Hi, what version have you tested? And can you check our main branch?

Nadock commented 1 year ago

This was tested on 1.51.6 and I've also been able to reproduce this when installing 06c50d7 from main, all using Python 3.11.1.

peterschutt commented 1 year ago

I actually came across the cause of this today, and it will be fixed for 2.0.

The handler gets a reference to its signature here:

https://github.com/starlite-api/starlite/blob/fbe51fb18bb685e579b66d0a42f1e0c3a4fae24f/starlite/handlers/http.py#L477

However, support for resolving types in inspect.Signature.from_callable() is only available in 3.10+.

We go on to use an isinstance() check on the return annotation, but it is still a string here:

https://github.com/starlite-api/starlite/blob/fbe51fb18bb685e579b66d0a42f1e0c3a4fae24f/starlite/handlers/http.py#L610

This will be fixed in 2.0 from this commit: https://github.com/starlite-api/starlite/pull/1160/commits/1ce9c5a84a0836139a2570317855b9a14d15d808 - and a few of the preceding commits on that branch.

We could do some eval() work to try to resolve the type of the return annotation in the 1.5x branch, but I'd suggest that you don't rely to heavily on __future__.annotations anywhere that there needs to be runtime introspection of types on the 1.x series.

Nadock commented 1 year ago

If there's a fix coming in 2.0 that's excellent 👏

We could do some eval() work to try to resolve the type of the return annotation in the 1.5x branch, but I'd suggest that you don't rely to heavily on future.annotations anywhere that there needs to be runtime introspection of types on the 1.x series.

Yeah that's fine honestly. This was my first foray into using Starlite so I the side effects of __future__.annotations weren't clear. Maybe worth a note in the documentation that __future__.annotations can cause issues, but it doesn't sound like back porting a fix is that worth it.