carltongibson / django-template-partials

Reusable named inline partials for the Django Template Language.
MIT License
387 stars 15 forks source link

Adding template_partials to INSTALLED_APPS breaks django-components #30

Open ekaj2 opened 6 months ago

ekaj2 commented 6 months ago

Our project currently has a few components in a root components/ directory similar to what's described here.

As soon as I add template_partials to INSTALLED_APPS, I get this when trying to access any page that uses django components:

    return render(request, "...myfile.html", context=context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../python3.11/site-packages/django/shortcuts.py", line 24, in render
    content = loader.render_to_string(template_name, context, request, using=using)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../python3.11/site-packages/django/template/loader.py", line 62, in render_to_string
    return template.render(context, request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ".../python3.11/site-packages/django/template/backends/django.py", line 63, in render
    reraise(exc, self.backend)
  File ".../python3.11/site-packages/django/template/backends/django.py", line 84, in reraise
    raise new from exc
django.template.exceptions.TemplateDoesNotExist: card_tailwind/card.html

I assume these two are clashing with the template loaders, but I don't understand the template loading system well enough to troubleshoot it much. Commenting out template_partials in INSTALLED_APPS immediately solves it. This is how TEMPLATES is currently setup:

TEMPLATES = [
    {
        'BACKEND': 'django.template.backends.django.DjangoTemplates',
        'DIRS': [
            BASE_DIR / "project_templates",
        ],
        # 'APP_DIRS': True,  # supposedly must be true for django-debug-toolbar, but seems to work like this (and it cannot be set when loaders is defined!): https://django-debug-toolbar.readthedocs.io/en/latest/installation.html#check-for-prerequisites
        'OPTIONS': {
            'context_processors': [
                'django.template.context_processors.debug',
                'django.template.context_processors.request',
                'django.contrib.auth.context_processors.auth',
                'django.contrib.messages.context_processors.messages',
                'django.template.context_processors.request',
            ],
            'loaders':[(
                'django.template.loaders.cached.Loader', [
                    'django.template.loaders.filesystem.Loader',
                    'django.template.loaders.app_directories.Loader',
                    'django_components.template_loader.Loader',
                ]
            )],
            'builtins': [
                'django_components.templatetags.component_tags',
            ]
        },
    },
]
carltongibson commented 6 months ago

Can you confirm you're using the latest version?

I haven't used Django components (yet) so I may need a bit of help digging in. For example, a minimal reproduce project would be helpful.

carltongibson commented 6 months ago

Maybe similar to #27 (resolved in latest version)

ekaj2 commented 6 months ago

Yes, on 23.4:

(reddy-training-app-py3.11) dj-proj ❯ pip freeze | grep "template"
django-template-partials==23.4

I'll work on a minimal repo for you now

ekaj2 commented 6 months ago

Okay here's a very minimal setup:

https://github.com/ekaj2/django-minimal-repro

I committed every few things, it's barely a couple additions to the polls demo: https://github.com/ekaj2/django-minimal-repro/commits/main/

Really appreciate the help

ekaj2 commented 6 months ago

Okay starting to take a stab at this.

In template_partials/loader.py, when I put a breakpoint on the yield on line 58, I see this:

Screenshot 2024-01-06 at 12 34 47 AM

Only two loaders, even though in config/settings.py, we have 3 loaders:

            'loaders': [(
                'django.template.loaders.cached.Loader', [
                    'django.template.loaders.filesystem.Loader',
                    'django.template.loaders.app_directories.Loader',
                    'django_components.template_loader.Loader',
                ]
            )],
            "context_processors": [

Now trying to figure out where that comes from and why it's chopping the last one.

Re: #27, I don't think that's the same thing here. If I understand correctly, that could have been mitigated by reordering the INSTALLED_APPS list, but that doesn't seem to have an effect here.

ekaj2 commented 6 months ago

Okay following this from the start. It looks like we start with django.py and the DjangoTemplate class init:

class DjangoTemplates(BaseEngine):
    app_dirname = "templates"

    def __init__(self, params):
        params = params.copy()
        options = params.pop("OPTIONS").copy()
        options.setdefault("autoescape", True)
        options.setdefault("debug", settings.DEBUG)
        options.setdefault("file_charset", "utf-8")
        libraries = options.get("libraries", {})
        options["libraries"] = self.get_templatetag_libraries(libraries)
        super().__init__(params)
        self.engine = Engine(self.dirs, self.app_dirs, **options)

Here, all 3 loaders are in options.

That gets passed to engine.py:

class Engine:
    default_builtins = [
        "django.template.defaulttags",
        "django.template.defaultfilters",
        "django.template.loader_tags",
    ]

    def __init__(
        self, ..., loaders=None, ...
    ):
        ...
        if loaders is None:
            loaders = ["django.template.loaders.filesystem.Loader"]
            if app_dirs:
                loaders += ["django.template.loaders.app_directories.Loader"]
            loaders = [("django.template.loaders.cached.Loader", loaders)]
        else:
            if app_dirs:
                raise ImproperlyConfigured(
                    "app_dirs must not be set when loaders is defined."
                )
        ...
        self.loaders = loaders
        ...

Still good.

But! DjangoTemplates actually gets called several times. When it gets called with template_partials.loader.Loader, it seems to have hardcoded these ones? ['django.template.loaders.filesystem.Loader', 'django.template.loaders.app_directories.Loader']

Ahah! apps.py puts these in...

Might not be able to do anything about it tonight

ekaj2 commented 6 months ago

Alright got the problem and it's easily solved by editing the config:

wrap_loaders checks if loaders[0][0] == "template_partials.loader.Loader". Which it doesn't if you had set TEMPLATES["OPTIONS"]["loaders"] to anything else. django-components suggests you do this in the installation section:

TEMPLATES = [
    {
        ...,
        'OPTIONS': {
            'context_processors': [
                ...
            ],
            'loaders':[(
                'django.template.loaders.cached.Loader', [
                    'django.template.loaders.filesystem.Loader',
                    'django.template.loaders.app_directories.Loader',
                    'django_components.template_loader.Loader',
                ]
            )],
        },
    },
]

So loaders[0][0] is actually "django.template.loaders.cached.Loader". Then wrap_loaders overrides it:

            already_configured = (
                loaders
                and isinstance(loaders, (list, tuple))
                and isinstance(loaders[0], tuple)
                and loaders[0][0] == "template_partials.loader.Loader"
            )
            if not already_configured:
                template_config.pop("APP_DIRS", None)
                default_loaders = [
                    "django.template.loaders.filesystem.Loader",
                    "django.template.loaders.app_directories.Loader",
                ]
                cached_loaders = [
                    ("django.template.loaders.cached.Loader", default_loaders)
                ]
                partial_loaders = [("template_partials.loader.Loader", cached_loaders)]
                template_config["OPTIONS"]["loaders"] = partial_loaders
            break

So the easy fix for anyone running into this is to wrap it yourself:

...
        "OPTIONS": {
            'loaders': [(
                'template_partials.loader.Loader', [(
                    'django.template.loaders.cached.Loader', [
                        'django.template.loaders.filesystem.Loader',
                        'django.template.loaders.app_directories.Loader',
                        'django_components.template_loader.Loader',
                    ])
                ]
            )],
...

But if that's all that's needed for the general case, should that be done by default by wrap_loaders?