Skip to content
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

Support multiple template directories for large projects #1214

Closed
nickleman opened this issue Jun 24, 2021 · 14 comments
Closed

Support multiple template directories for large projects #1214

nickleman opened this issue Jun 24, 2021 · 14 comments

Comments

@nickleman
Copy link

I would like to be able to have multiple directories included in the Jinja2Template response. This allows for well structured applications with sub-application separation. For example in flask the individual blueprints can register their own template directory and the templates in that directory can also reference/include templates from the parent project on up. This enables having base templates with the look and feel of the site at the parent level and then the page content in the sub-project.

There is an implementation out there (https://github.com/accent-starlette/starlette-core/blob/master/starlette_core/templating.py) but I'd like to see it built in. Especially since the referenced implementation isn't in PyPI.

@alex-oleshkevich
Copy link
Member

I'd rather add an option to pass jinja environment object to the Jinja2Template constructor so we could have full control over it.

@nickleman
Copy link
Author

That works just fine for me. Maybe get the environment from the parent instance and pass it to the constructor?

@aminalaee
Copy link
Member

aminalaee commented Jun 26, 2021

@alex-oleshkevich Wouldn't it be easier to just pass loader to the Jinja2Template so the caller can implement the loader but keep the env in Jinja2Templates itself?

@nickleman
Copy link
Author

Or pass the "parent" ninja template instance top the child one along with the new directory.

@alex-oleshkevich
Copy link
Member

alex-oleshkevich commented Jun 26, 2021

Loader is only one of multiple options that Jinja's Environment class expects. There are also extensions, caching, escaping, and many other options you may want to customize time to time. See https://jinja.palletsprojects.com/en/3.0.x/api/#jinja2.Environment

Personally, I'd prefer to have a fine grained control over the environment class. As a shortcut, we can also accept template dirs or loader instance. This is discussable.

We can also get env class with get_env method and configure it ad-hoc, but, IMO, this is not a nice API.

@aminalaee
Copy link
Member

aminalaee commented Jun 26, 2021

I agree with the Environment argument approach.

Persoanally I'd prefer the loader argument to the list of templates shortcut because leaving the loader implementation to the caller would be cleaner.

@graingert
Copy link
Member

fyi there's https://accent-starlette.github.io/starlette-core/templating/ that provides a loader kwarg

@aminalaee
Copy link
Member

aminalaee commented Jan 9, 2022

We can achieve this with #1401:

loader = jinja2.FileSystemLoader(directory)

templates = Jinja2Templates(
        directory=directory, loader=loader
)

Any feedback would be appreciated

@nickleman
Copy link
Author

@aminalaee I'm not a fan of the redundancy of the directory argument. Seems unnecessary. I can't remember at the moment and can't check it right now, but if I grab the file system loader from the parent, can I then add a directory? This discussion was all about adding additional directories to the existing (parent) templates so that sub-apps can maintain templates within their own area and still reference off to a base template, etc.

@aminalaee
Copy link
Member

aminalaee commented Jan 9, 2022

@nickleman I don't like the redundant directory keyword either as described in the PR. But until we change the Jinja2Templates signature, I don't think there's anything to do about it.

I think this will achieve what you're looking for. But maybe not as you compared it to the Flask version.

You can setup a custom jinja2 loader to handle your complex structure of files and directories. So maybe a directory with it's sub dorectories per application as you want. Then use that Jinja2Templates instance to render the templates.

The issue discussed was that there was no API to allow you set a custom loader or really any other options for the Jinja2 Environment.

@aminalaee
Copy link
Member

aminalaee commented Jan 19, 2022

After #1401, as a workaround this can be achieved with:

import jinja2
from starlette.templating import Jinja2Templates


loader = jinja2.ChoiceLoader(
            [
                jinja2.FileSystemLoader("templates"),
                jinja2.PackageLoader("example", "templates"),
            ]
        )

templates = Jinja2Templates(directory="templates", loader=loader)

This isn't ideal as directory will be redundant, maybe later on this can be improved by allowing a different signature for Jinja2Templates with a single directory.

@alex-oleshkevich
Copy link
Member

Personally, I dislike that special case for the loader. I understand that it is, probably, the most used use-case but, IMO, we should consider passing the jinja2.Environment instead of loader for much cleaner and flexible API.
If you look at Environment's constructor argument's you will see plenty of options. If someone would want to change one of them he will be in exactly the same situation as we are now with template directories :)

@aminalaee
Copy link
Member

I think the special case doesn't have much to do with loader. It's because of the redundant directory argument that makes loader special.

Even if instead of Environment options, we accepted Environment instance itself, we would still be in the same situation.

If you look at Environment's constructor argument's you will see plenty of options. If someone would want to change one of them he will be in exactly the same situation as we are now with template directories :)

I don't really think I understand this correctly. Can you explain a bit?

@tomchristie
Copy link
Member

Given that Starlette's Jinja2Template class is really a fairly small amount of code, it's reasonable for us to say - "this is what we choose to do here. If you'd like to approach this differently then do so within your own codebase".

Constraints are a good thing. Having a single template directory is a perfectly okay design choice for Starlette itself to make, and at this point in time it's how I'd like it to stay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants