volfpeter / fasthx

FastAPI server-side rendering with built-in HTMX support.
https://volfpeter.github.io/fasthx/
MIT License
458 stars 9 forks source link

Pass endpoint dependencies to Jinja context via request.state #6

Closed hasansezertasan closed 10 months ago

hasansezertasan commented 10 months ago

Related to #4

What is request.state and why am I using it?

https://github.com/encode/starlette/blob/433da65fc1431754dae0c3c87290b0421aa4a6f9/starlette/datastructures.py#L680-L685

Request State documentation says: An object that can be used to store arbitrary state.

I saw many implementations using the request.state to carry database sessions, current user, client sessions, etc. I think it's the best way to pass endpoint dependencies to the Jinja context without affecting anything.

volfpeter commented 10 months ago

What if someone already uses depends in there? It may be unlikely, but it's still not something a package should do. Also, its usage wouldn't be obvious to people, because its a bit of hidden magic. Explicit is better than implicit. I'll post my recommended solution in the issue.

hasansezertasan commented 10 months ago

What if someone already uses depends in there? It may be unlikely, but it's still not something a package should do.

I disagree, there are quite a lot of (popular) packages that follow this approach but you are right, depends might be used by another package, and still, we are passing that data to the Jinja Template file and returning the HTML response.

Also, its usage wouldn't be obvious to people, because its a bit of hidden magic. Explicit is better than implicit. I'll post my recommended solution in the issue.

You are right about that but we can document it, the only advantage of this approach is to provide a way to access the dependencies without overriding the Jinja class.

hasansezertasan commented 10 months ago

I'd like to mention this comment from the author of starlette: https://github.com/encode/starlette/issues/659#issuecomment-539541003

The recommended way is this: "For per-request information you can use request.state to store any additional information."

Other related issues:

So this is actually the recommended way of doing such a thing.

volfpeter commented 10 months ago

This may be the recommended way for certain use-cases in application development (always opinionated code and always in the hand of its developer), but I don't think it's good behavior for a Python package, because it could break applications.

hasansezertasan commented 10 months ago

I also want to highlight these resources:

I see that this is the "official" recommendation from the Starlette dev team. I also want to highlight some third-party-extensions that use request.state.something to store pre-request state/data.

Some other resources:

I believe there is no harm in using request.state._depends and update the README accordingly (in case someone uses that keyword for something else, it'll be a warning).

In the end, what do application and request context even exist for?