Prevent duplicates when collecting template assets in asset templatetag (#119)

Author: joshuadavidthomas

CHANGELOG.md

@@ -18,6 +18,14 @@ and this project attempts to adhere to [Semantic Versioning](https://semver.org/
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed the potential for duplicate asset tags in the `{% bird:css %}` and `{% bird:js %}` templatetags by using a `set` instead of a `list` when collecting a template's component assets.
+
+### Changed
+
+- **Internal**: Refactored asset rendering logic by centralizing tag name parsing and HTML tag generation to the `django_bird.staticfiles` module.
+
 ## [0.10.2]
 
 ### Changed

src/django_bird/staticfiles.py

@@ -26,6 +26,20 @@ def content_type(self):
     def ext(self):
         return f".{self.value}"
 
+    @classmethod
+    def from_tag_name(cls, tag_name: str):
+        try:
+            asset_type = tag_name.split(":")[1]
+            match asset_type:
+                case "css":
+                    return cls.CSS
+                case "js":
+                    return cls.JS
+                case _:
+                    raise ValueError(f"Unknown asset type: {asset_type}")
+        except IndexError as e:
+            raise ValueError(f"Invalid tag name: {tag_name}") from e
+
 
 @dataclass(frozen=True, slots=True)
 class Asset:
@@ -39,6 +53,13 @@ def __hash__(self) -> int:
     def exists(self) -> bool:
         return self.path.exists()
 
+    def render(self):
+        match self.type:
+            case AssetType.CSS:
+                return f'<link rel="stylesheet" href="{self.url}">'
+            case AssetType.JS:
+                return f'<script src="{self.url}"></script>'
+
     @property
     def url(self) -> str:
         component_name = self.path.stem

src/django_bird/templatetags/tags/asset.py

@@ -1,69 +1,40 @@
 # pyright: reportAny=false
 from __future__ import annotations
 
+from typing import final
+
 from django import template
 from django.template.base import Parser
 from django.template.base import Token
 from django.template.context import Context
 
-from django_bird._typing import TagBits
 from django_bird._typing import override
 from django_bird.components import components
-from django_bird.staticfiles import Asset
 from django_bird.staticfiles import AssetType
 
 CSS_TAG = "bird:css"
 JS_TAG = "bird:js"
 
 
-def do_asset(parser: Parser, token: Token) -> AssetNode:
+def do_asset(_parser: Parser, token: Token) -> AssetNode:
     bits = token.split_contents()
-    asset_type = parse_asset_type(bits)
-    return AssetNode(asset_type)
-
-
-def parse_asset_type(bits: TagBits) -> AssetType:
     if len(bits) < 1:
         msg = "bird:assets tag requires at least one argument"
         raise template.TemplateSyntaxError(msg)
-
     tag_name = bits[0]
-
-    try:
-        asset_type = tag_name.split(":")[1]
-        match asset_type:
-            case "css":
-                return AssetType.CSS
-            case "js":
-                return AssetType.JS
-            case _:
-                raise ValueError(f"Unknown asset type: {asset_type}")
-    except IndexError as e:
-        raise ValueError(f"Invalid tag name: {tag_name}") from e
+    asset_type = AssetType.from_tag_name(tag_name)
+    return AssetNode(asset_type)
 
 
+@final
 class AssetNode(template.Node):
     def __init__(self, asset_type: AssetType):
         self.asset_type = asset_type
 
     @override
     def render(self, context: Context) -> str:
-        component_assets = components.get_assets(self.asset_type)
-        return self._render_assets(component_assets)
-
-    def _render_assets(self, assets: list[Asset]) -> str:
+        assets = components.get_assets(self.asset_type)
         if not assets:
             return ""
-
-        if self.asset_type == AssetType.CSS:
-            tags = (
-                f'<link rel="stylesheet" href="{asset.url}">'
-                for asset in sorted(assets, key=lambda a: a.path)
-            )
-        else:  # JS
-            tags = (
-                f'<script src="{asset.url}"></script>'
-                for asset in sorted(assets, key=lambda a: a.path)
-            )
-
-        return "\n".join(tags)
+        rendered = {asset.render() for asset in sorted(assets, key=lambda a: a.path)}
+        return "\n".join(rendered)

tests/templatetags/test_asset.py

@@ -204,6 +204,61 @@ def test_component_render_order(self, create_template, templates_dir):
             in rendered[body_start:]
         )
 
+    def test_asset_duplication(self, create_template, templates_dir):
+        alert = TestComponent(
+            name="alert", content='<div class="alert">{{ slot }}</div>'
+        ).create(templates_dir)
+        alert_css = TestAsset(
+            component=alert, content=".alert { color: red; }", asset_type=AssetType.CSS
+        ).create()
+        alert_js = TestAsset(
+            component=alert, content="console.log('alert');", asset_type=AssetType.JS
+        ).create()
+
+        base_path = templates_dir / "base.html"
+        base_path.write_text("""
+            <html>
+            <head>
+                <title>Test</title>
+                {% bird:css %}
+            </head>
+            <body>
+                {% bird alert %}Base Alert{% endbird %}
+                {% bird alert %}Base Alert{% endbird %}
+                {% bird alert %}Base Alert{% endbird %}
+                {% block content %}{% endblock %}
+                {% bird:js %}
+            </body>
+            </html>
+        """)
+
+        child_path = templates_dir / "child.html"
+        child_path.write_text("""
+            {% extends 'base.html' %}
+            {% block content %}
+                {% bird alert %}Base Alert{% endbird %}
+                {% bird alert %}Base Alert{% endbird %}
+                {% bird alert %}Base Alert{% endbird %}
+            {% endblock %}
+        """)
+
+        template = create_template(child_path)
+
+        rendered = template.render({})
+
+        assert (
+            rendered.count(
+                f'<link rel="stylesheet" href="/__bird__/assets/{alert.name}/{alert_css.file.name}">'
+            )
+            == 1
+        )
+        assert (
+            rendered.count(
+                f'<script src="/__bird__/assets/{alert.name}/{alert_js.file.name}"></script>'
+            )
+            == 1
+        )
+
 
 class TestNode:
     def test_no_template(self):

tests/test_staticfiles.py

@@ -32,6 +32,18 @@ def test_exists_nonexistent(self):
         missing_asset = Asset(Path("missing.css"), AssetType.CSS)
         assert missing_asset.exists() is False
 
+    @pytest.mark.parametrize(
+        "asset,expected_html_tag_bits",
+        [
+            (Asset(Path("static.css"), AssetType.CSS), 'link rel="stylesheet" href='),
+            (Asset(Path("static.js"), AssetType.JS), "script src="),
+        ],
+    )
+    def test_render(self, asset, expected_html_tag_bits):
+        rendered = asset.render()
+        assert expected_html_tag_bits in rendered
+        assert asset.url in rendered
+
     @pytest.mark.parametrize(
         "path,expected",
         [