Repository: joshuadavidthomas/django-bird

Issue:
Deprecate and remove `BirdLoader` and `Compiler`
Given the recent changes to `ComponentRegistry.discover_components` in #148 and how I'm leaning about using dj-angles for #15, I don't think the custom template loader is really need anymore.

Initially I added the custom template loader to be able to be able to use custom html-like tags that could get compiled down to regular Django template syntax, but given I can basically accomplish all I want using dj-angles, there's not really a reason to do it in this library.

Also, there were some limitations I ran into loading the components and making sure all of the assets were attached to the template being rendered -- namely, around the order templates are rendered in conjunction with templates using the `{% extends %}` and `{% includes %}` templatetags. I was trying to drive a nail with a sledgehammer (custom loader to find assets at render time) instead of just using a simple hammer (regex scanning the directories at app initialization). So instead of just loading the components, I changed the logic of `discover_components` to scan *all* templates in a project for any uses of a `{% bird %}` tag. (Still a *bit* worried about the performance of that, but I can get to that later if it becomes an issue.)

The asset loading has already been removed, but the loader is still sticking around, "compiling" and caching the loaded templates -- but basically doing nothing:

https://github.com/joshuadavidthomas/django-bird/blob/4953506ed4f69f03d516a437ecb9239cc0347c3e/src/django_bird/loader.py#L18-L35

https://github.com/joshuadavidthomas/django-bird/blob/4953506ed4f69f03d516a437ecb9239cc0347c3e/src/django_bird/compiler.py#L10-L24

The compiler can just be deleted straight-up, but the loader will need to be deprecated for at least one version to give anyone who's not using the auto-configuration to update their settings.

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2ebb60b..7bd236d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -32,6 +32,7 @@ and this project attempts to adhere to [Semantic Versioning](https://semver.org/
 ### Deprecated
 
 - The built-in asset serving view (`asset_view`) is deprecated and will be removed in the next minor version (v0.14.0). Use `BirdAssetFinder` with Django's staticfiles app.
+- The `BirdLoader` template loader is deprecated and will be removed in a future version. If you have enabled manual configuration by setting `ENABLE_AUTO_CONFIG=False`, please remove `django_bird.loader.BirdLoader` from your `TEMPLATES` setting.
 
 ### Removed
 
diff --git a/docs/configuration.md b/docs/configuration.md
index c6864a2..4e12993 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -70,7 +70,6 @@ If you would like to disable this behavior and perform the setup manually, setti
 When `ENABLE_AUTO_CONFIG` is set to `False`, you need to manually configure the following:
 
 1. Add django-bird's template tags to Django's built-ins.
-2. Include django-bird's loader in your template loaders, ensuring it comes before Django's default filesystem and app directories loaders.
 
 The complete setup in your settings file should look like this:
 
@@ -95,22 +94,12 @@ TEMPLATES = [
             "builtins": [
                 "django_bird.templatetags.django_bird",
             ],
-            "loaders": [
-                (
-                    "django.template.loaders.cached.Loader",
-                    [
-                        "django_bird.loader.BirdLoader",
-                        "django.template.loaders.filesystem.Loader",
-                        "django.template.loaders.app_directories.Loader",
-                    ],
-                ),
-            ],
         },
     }
 ]
 ```
 
-This configuration ensures that django-bird's templatetags are available globally and that its loader is used to compile bird component templates before the standard Django loaders.
+This configuration ensures that django-bird's templatetags are available globally.
 
 ## `ENABLE_BIRD_ATTRS`
 
diff --git a/src/django_bird/conf.py b/src/django_bird/conf.py
index a67aa66..4505b53 100644
--- a/src/django_bird/conf.py
+++ b/src/django_bird/conf.py
@@ -17,7 +17,6 @@ DJANGO_BIRD_SETTINGS_NAME = "DJANGO_BIRD"
 
 DJANGO_BIRD_BUILTINS = "django_bird.templatetags.django_bird"
 DJANGO_BIRD_FINDER = "django_bird.staticfiles.BirdAssetFinder"
-DJANGO_BIRD_LOADER = "django_bird.loader.BirdLoader"
 
 
 @dataclass
@@ -70,7 +69,6 @@ class AutoConfigurator:
 
         options = template_config.setdefault("OPTIONS", {})
 
-        self.configure_loaders(options)
         self.configure_builtins(options)
 
         # Force re-evaluation of settings.TEMPLATES because EngineHandler caches it.
@@ -78,25 +76,6 @@ class AutoConfigurator:
             del django.template.engines.templates
             django.template.engines._engines = {}  # type: ignore[attr-defined]
 
-    def configure_loaders(self, options: dict[str, Any]) -> None:
-        loaders = options.setdefault("loaders", [])
-
-        # find the inner-most loaders, which is an iterable of only strings
-        while not all(isinstance(loader, str) for loader in loaders):
-            for loader in loaders:
-                # if we've found a list or tuple, we aren't yet in the inner-most loaders
-                if isinstance(loader, list | tuple):
-                    # reassign `loaders` variable to force the while loop restart
-                    loaders = loader
-
-        # if django-bird's loader is the first, we good
-        loaders_already_configured = (
-            len(loaders) > 0 and DJANGO_BIRD_LOADER == loaders[0]
-        )
-
-        if not loaders_already_configured:
-            loaders.insert(0, DJANGO_BIRD_LOADER)
-
     def configure_builtins(self, options: dict[str, Any]) -> None:
         builtins = options.setdefault("builtins", [])
 
diff --git a/src/django_bird/loader.py b/src/django_bird/loader.py
index d70e093..a9ad179 100644
--- a/src/django_bird/loader.py
+++ b/src/django_bird/loader.py
@@ -1,16 +1,18 @@
 from __future__ import annotations
 
-from django.template.base import Origin
+import warnings
+
 from django.template.engine import Engine
 from django.template.loaders.filesystem import Loader as FileSystemLoader
 
-from ._typing import override
-
 
 class BirdLoader(FileSystemLoader):
     def __init__(self, engine: Engine):
+        warnings.warn(
+            "BirdLoader is deprecated and will be removed in a future version. "
+            "Please remove 'django_bird.loader.BirdLoader' from your TEMPLATES setting "
+            "in settings.py and use Django's built-in 'django.template.loaders.filesystem.Loader' instead.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
         super().__init__(engine)
-
-    @override
-    def get_contents(self, origin: Origin) -> str:
-        return super().get_contents(origin)


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index afb4e17..141fffd 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -48,7 +48,6 @@ TEST_SETTINGS = {
                     "django_bird.templatetags.django_bird",
                 ],
                 "loaders": [
-                    "django_bird.loader.BirdLoader",
                     "django.template.loaders.filesystem.Loader",
                     "django.template.loaders.app_directories.Loader",
                 ],
@@ -127,7 +126,6 @@ def create_template():
         engine = Engine(
             builtins=["django_bird.templatetags.django_bird"],
             dirs=[str(template_file.parent)],
-            loaders=["django_bird.loader.BirdLoader"],
         )
         template = engine.get_template(template_file.name)
         backend = DjangoTemplates(
diff --git a/tests/test_conf.py b/tests/test_conf.py
index b3b5e1f..61d253f 100644
--- a/tests/test_conf.py
+++ b/tests/test_conf.py
@@ -2,14 +2,13 @@ from __future__ import annotations
 
 import pytest
 from django.conf import settings
-from django.core.exceptions import ImproperlyConfigured
 from django.template.base import Template
 from django.template.context import Context
+from django.template.exceptions import TemplateDoesNotExist
 from django.test import override_settings
 
 from django_bird.conf import DJANGO_BIRD_BUILTINS
 from django_bird.conf import DJANGO_BIRD_FINDER
-from django_bird.conf import DJANGO_BIRD_LOADER
 from django_bird.conf import AppSettings
 from django_bird.conf import AutoConfigurator
 from django_bird.conf import app_settings
@@ -30,7 +29,6 @@ class TestAutoConfigurator:
     def reset_settings(self):
         template_options = settings.TEMPLATES[0]["OPTIONS"]
 
-        assert DJANGO_BIRD_LOADER in template_options["loaders"]
         assert DJANGO_BIRD_BUILTINS in template_options["builtins"]
         assert DJANGO_BIRD_FINDER in settings.STATICFILES_FINDERS
 
@@ -45,11 +43,6 @@ class TestAutoConfigurator:
                 | {
                     **settings.TEMPLATES[0],
                     "OPTIONS": {
-                        "loaders": [
-                            loader
-                            for loader in template_options["loaders"]
-                            if loader != DJANGO_BIRD_LOADER
-                        ],
                         "builtins": [
                             builtin
                             for builtin in template_options["builtins"]
@@ -61,7 +54,6 @@ class TestAutoConfigurator:
         ):
             options = settings.TEMPLATES[0]["OPTIONS"]
 
-            assert DJANGO_BIRD_LOADER not in options["loaders"]
             assert DJANGO_BIRD_BUILTINS not in options["builtins"]
             assert DJANGO_BIRD_FINDER not in settings.STATICFILES_FINDERS
 
@@ -72,7 +64,6 @@ class TestAutoConfigurator:
 
         configurator.autoconfigure()
 
-        assert DJANGO_BIRD_LOADER in template_options["loaders"]
         assert DJANGO_BIRD_BUILTINS in template_options["builtins"]
         assert DJANGO_BIRD_FINDER in settings.STATICFILES_FINDERS
 
@@ -86,17 +77,9 @@ class TestAutoConfigurator:
         template_options = settings.TEMPLATES[0]["OPTIONS"]
 
         assert app_settings.ENABLE_AUTO_CONFIG is False
-        assert DJANGO_BIRD_LOADER not in template_options["loaders"]
         assert DJANGO_BIRD_BUILTINS not in template_options["builtins"]
         assert DJANGO_BIRD_FINDER not in settings.STATICFILES_FINDERS
 
-    def test_configure_loaders(self, configurator):
-        template_options = settings.TEMPLATES[0]["OPTIONS"]
-
-        configurator.configure_loaders(template_options)
-
-        assert DJANGO_BIRD_LOADER in template_options["loaders"]
-
     def test_configure_builtins(self, configurator):
         template_options = settings.TEMPLATES[0]["OPTIONS"]
 
@@ -116,209 +99,6 @@ class TestAutoConfigurator:
 
         assert configurator._configured is True
 
-    @pytest.mark.parametrize(
-        "init_options,expected",
-        [
-            (
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                    ],
-                    "loaders": [
-                        "django.template.loaders.filesystem.Loader",
-                        "django.template.loaders.app_directories.Loader",
-                    ],
-                },
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                        "django_bird.templatetags.django_bird",
-                    ],
-                    "loaders": [
-                        "django_bird.loader.BirdLoader",
-                        "django.template.loaders.filesystem.Loader",
-                        "django.template.loaders.app_directories.Loader",
-                    ],
-                },
-            ),
-            (
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                    ],
-                    "loaders": [
-                        (
-                            "django.template.loaders.cached.Loader",
-                            [
-                                "django.template.loaders.filesystem.Loader",
-                                "django.template.loaders.app_directories.Loader",
-                            ],
-                        ),
-                    ],
-                },
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                        "django_bird.templatetags.django_bird",
-                    ],
-                    "loaders": [
-                        (
-                            "django.template.loaders.cached.Loader",
-                            [
-                                "django_bird.loader.BirdLoader",
-                                "django.template.loaders.filesystem.Loader",
-                                "django.template.loaders.app_directories.Loader",
-                            ],
-                        ),
-                    ],
-                },
-            ),
-            (
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                    ],
-                    "loaders": [
-                        (
-                            "template_partials.loader.Loader",
-                            [
-                                (
-                                    "django.template.loaders.cached.Loader",
-                                    [
-                                        "django.template.loaders.filesystem.Loader",
-                                        "django.template.loaders.app_directories.Loader",
-                                    ],
-                                ),
-                            ],
-                        )
-                    ],
-                },
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                        "django_bird.templatetags.django_bird",
-                    ],
-                    "loaders": [
-                        (
-                            "template_partials.loader.Loader",
-                            [
-                                (
-                                    "django.template.loaders.cached.Loader",
-                                    [
-                                        "django_bird.loader.BirdLoader",
-                                        "django.template.loaders.filesystem.Loader",
-                                        "django.template.loaders.app_directories.Loader",
-                                    ],
-                                ),
-                            ],
-                        )
-                    ],
-                },
-            ),
-            (
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                        "django_cotton.templatetags.cotton",
-                    ],
-                    "loaders": [
-                        (
-                            "django.template.loaders.cached.Loader",
-                            [
-                                "django_cotton.cotton_loader.Loader",
-                                "django.template.loaders.filesystem.Loader",
-                                "django.template.loaders.app_directories.Loader",
-                            ],
-                        ),
-                    ],
-                },
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                        "django_cotton.templatetags.cotton",
-                        "django_bird.templatetags.django_bird",
-                    ],
-                    "loaders": [
-                        (
-                            "django.template.loaders.cached.Loader",
-                            [
-                                "django_bird.loader.BirdLoader",
-                                "django_cotton.cotton_loader.Loader",
-                                "django.template.loaders.filesystem.Loader",
-                                "django.template.loaders.app_directories.Loader",
-                            ],
-                        ),
-                    ],
-                },
-            ),
-            (
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                        "django_cotton.templatetags.cotton",
-                    ],
-                    "loaders": [
-                        (
-                            "template_partials.loader.Loader",
-                            [
-                                (
-                                    "django.template.loaders.cached.Loader",
-                                    [
-                                        "django_cotton.cotton_loader.Loader",
-                                        "django.template.loaders.filesystem.Loader",
-                                        "django.template.loaders.app_directories.Loader",
-                                    ],
-                                ),
-                            ],
-                        )
-                    ],
-                },
-                {
-                    "builtins": [
-                        "django.template.defaulttags",
-                        "django_cotton.templatetags.cotton",
-                        "django_bird.templatetags.django_bird",
-                    ],
-                    "loaders": [
-                        (
-                            "template_partials.loader.Loader",
-                            [
-                                (
-                                    "django.template.loaders.cached.Loader",
-                                    [
-                                        "django_bird.loader.BirdLoader",
-                                        "django_cotton.cotton_loader.Loader",
-                                        "django.template.loaders.filesystem.Loader",
-                                        "django.template.loaders.app_directories.Loader",
-                                    ],
-                                ),
-                            ],
-                        )
-                    ],
-                },
-            ),
-        ],
-    )
-    def test_template_settings(self, init_options, expected, configurator):
-        with override_settings(
-            TEMPLATES=[
-                settings.TEMPLATES[0]
-                | {
-                    **settings.TEMPLATES[0],
-                    "OPTIONS": init_options,
-                }
-            ]
-        ):
-            template_options = settings.TEMPLATES[0]["OPTIONS"]
-
-            assert template_options == init_options
-            assert DJANGO_BIRD_LOADER not in template_options["loaders"]
-            assert DJANGO_BIRD_BUILTINS not in template_options["builtins"]
-
-            configurator.autoconfigure()
-
-            assert template_options == expected
-
     @override_settings(
         **{
             "STATICFILES_FINDERS": [
@@ -346,12 +126,8 @@ class TestAutoConfigurator:
 
         template_options = settings.TEMPLATES[0]["OPTIONS"]
 
-        assert DJANGO_BIRD_LOADER in template_options["loaders"]
         assert DJANGO_BIRD_BUILTINS in template_options["builtins"]
         assert DJANGO_BIRD_FINDER in settings.STATICFILES_FINDERS
 
-        with pytest.raises(
-            ImproperlyConfigured,
-            match="app_dirs must not be set when loaders is defined.",
-        ):
+        with pytest.raises(TemplateDoesNotExist, match=example_template.base.name):
             Template(example_template.content).render(Context({}))
diff --git a/tests/test_loader.py b/tests/test_loader.py
index dde74f2..ce3dca3 100644
--- a/tests/test_loader.py
+++ b/tests/test_loader.py
@@ -1,74 +1,18 @@
 from __future__ import annotations
 
 import pytest
-from django.conf import settings
-from django.template.loader import get_template
-from django.test import override_settings
+from django.template.engine import Engine
 
+from django_bird.loader import BirdLoader
 
-@pytest.mark.parametrize(
-    "template_name",
-    (
-        "with_bird.html",
-        "without_bird.html",
-    ),
-)
-def test_render_template(template_name):
-    context = {
-        "title": "Test Title",
-        "content": "Test Content",
-    }
 
-    template = get_template(template_name)
-    rendered = template.render(context)
-
-    assert rendered
-
-
-@pytest.mark.parametrize(
-    "loaders",
-    [
-        [
-            "django_bird.loader.BirdLoader",
-            "django.template.loaders.filesystem.Loader",
-            "django.template.loaders.app_directories.Loader",
-        ],
-        [
-            "django.template.loaders.filesystem.Loader",
-            "django_bird.loader.BirdLoader",
-            "django.template.loaders.app_directories.Loader",
-        ],
-        [
-            "django.template.loaders.filesystem.Loader",
-            "django.template.loaders.app_directories.Loader",
-            "django_bird.loader.BirdLoader",
-        ],
-    ],
-)
-def test_loader_order(loaders, example_template, registry):
-    registry.discover_components()
-
-    with override_settings(
-        TEMPLATES=[
-            settings.TEMPLATES[0]
-            | {
-                **settings.TEMPLATES[0],
-                "OPTIONS": {
-                    **settings.TEMPLATES[0]["OPTIONS"],
-                    "loaders": loaders,
-                },
-            }
-        ]
+def test_bird_loader_deprecation_warning():
+    with pytest.warns(
+        DeprecationWarning,
+        match=(
+            "BirdLoader is deprecated and will be removed in a future version. "
+            "Please remove 'django_bird.loader.BirdLoader' from your TEMPLATES setting "
+            "in settings.py and use Django's built-in 'django.template.loaders.filesystem.Loader' instead."
+        ),
     ):
-        rendered = get_template(example_template.template.name).render({})
-
-    assert all(
-        asset.url in rendered
-        for component in example_template.used_components
-        for asset in component.assets
-    )
-    assert not any(
-        asset.url in rendered
-        for component in example_template.unused_components
-        for asset in component.assets
-    )
+        BirdLoader(Engine())