fix: loader tags compatibility (#468)

docs/slots_and_blocks.md

@@ -0,0 +1,166 @@
+# Using `slot` and `block` tags
+
+1.  First let's clarify how `include` and `extends` tags work inside components.
+    So when component template includes `include` or `extends` tags, it's as if the "included"
+    template was inlined. So if the "included" template contains `slot` tags, then the component
+    uses those slots.
+
+        So if you have a template `abc.html`:
+        ```django
+        <div>
+          hello
+          {% slot "body" %}{% endslot %}
+        </div>
+        ```
+
+        And components that make use of `abc.html` via `include` or `extends`:
+        ```py
+        @component.register("my_comp_extends")
+        class MyCompWithExtends(component.Component):
+            template = """{% extends "abc.html" %}"""
+
+        @component.register("my_comp_include")
+        class MyCompWithInclude(component.Component):
+            template = """{% include "abc.html" %}"""
+        ```
+
+        Then you can set slot fill for the slot imported via `include/extends`:
+
+        ```django
+        {% component "my_comp_extends" %}
+        	{% fill "body" %}
+        		123
+        	{% endfill %}
+        {% endcomponent %}
+        ```
+
+        And it will render:
+        ```html
+        <div>
+          hello
+          123
+        </div>
+        ```
+
+2.  Slot and block
+
+    So if you have a template `abc.html` like so:
+
+    ```django
+    <div>
+      hello
+      {% block inner %}
+        1
+        {% slot "body" %}
+          2
+        {% endslot %}
+      {% endblock %}
+    </div>
+    ```
+
+    and component `my_comp`:
+
+    ```py
+    @component.register("my_comp")
+    class MyComp(component.Component):
+    	template_name = "abc.html"
+    ```
+
+    Then:
+
+    1.  Since the `block` wasn't overriden, you can use the `body` slot:
+
+        ```django
+        {% component "my_comp" %}
+        	{% fill "body" %}
+        		XYZ
+        	{% endfill %}
+        {% endcomponent %}
+        ```
+
+        And we get:
+
+        ```html
+        <div>hello 1 XYZ</div>
+        ```
+
+    2.  `blocks` CANNOT be overriden through the `component` tag, so something like this:
+
+        ```django
+        {% component "my_comp" %}
+        	{% fill "body" %}
+        		XYZ
+        	{% endfill %}
+        {% endcomponent %}
+        {% block "inner" %}
+        	456
+        {% endblock %}
+        ```
+
+        Will still render the component content just the same:
+
+        ```html
+        <div>hello 1 XYZ</div>
+        ```
+
+    3.  You CAN override the `block` tags of `abc.html` if my component template
+        uses `extends`. In that case, just as you would expect, the `block inner` inside
+        `abc.html` will render `OVERRIDEN`:
+
+        ````py
+        @component.register("my_comp")
+        class MyComp(component.Component):
+        template_name = """
+        {% extends "abc.html" %}
+
+            		{% block inner %}
+            			OVERRIDEN
+            		{% endblock %}
+            	"""
+            ```
+
+        ````
+
+    4.  This is where it gets interesting (but still intuitive). You can insert even
+        new `slots` inside these "overriding" blocks:
+
+        ```py
+        @component.register("my_comp")
+        class MyComp(component.Component):
+        	template_name = """
+        		{% extends "abc.html" %}
+
+        		{% load component_tags %}
+        		{% block "inner" %}
+        			OVERRIDEN
+        			{% slot "new_slot" %}
+        				hello
+        			{% endslot %}
+        		{% endblock %}
+        	"""
+        ```
+
+        And you can then pass fill for this `new_slot` when rendering the component:
+
+        ```django
+        {% component "my_comp" %}
+        	{% fill "new_slot" %}
+        		XYZ
+        	{% endfill %}
+        {% endcomponent %}
+        ```
+
+        NOTE: Currently you can supply fills for both `new_slot` and `body` slots, and you will
+        not get an error for an invalid/unknown slot name. But since `body` slot is not rendered,
+        it just won't do anything. So this renders the same as above:
+
+        ```django
+        {% component "my_comp" %}
+        	{% fill "new_slot" %}
+        		XYZ
+        	{% endfill %}
+        	{% fill "body" %}
+        		www
+        	{% endfill %}
+        {% endcomponent %}
+        ```

src/django_components/component.py

@@ -9,6 +9,7 @@ from django.forms.widgets import Media, MediaDefiningClass
 from django.http import HttpResponse
 from django.template.base import FilterExpression, Node, NodeList, Template, TextNode
 from django.template.context import Context
+from django.template.exceptions import TemplateSyntaxError
 from django.template.loader import get_template
 from django.utils.html import escape
 from django.utils.safestring import SafeString, mark_safe
@@ -291,6 +292,7 @@ class Component(View, metaclass=SimplifiedInterfaceMediaDefiningClass):
             context_data = {}
 
         slots, resolved_fills = resolve_slots(
+            context,
             template,
             component_name=self.registered_name,
             context_data=context_data,
@@ -402,6 +404,12 @@ class ComponentNode(Node):
                 # Note that outer component context is used to resolve variables in
                 # fill tag.
                 resolved_name = fill_node.name_fexp.resolve(context)
+                if resolved_name in fill_content:
+                    raise TemplateSyntaxError(
+                        f"Multiple fill tags cannot target the same slot name: "
+                        f"Detected duplicate fill tag name '{resolved_name}'."
+                    )
+
                 resolved_fill_alias = fill_node.resolve_alias(context, resolved_component_name)
                 fill_content[resolved_name] = FillContent(fill_node.nodelist, resolved_fill_alias)
 

src/django_components/node.py

@@ -1,7 +1,9 @@
 from typing import Callable, List, NamedTuple, Optional
 
+from django.template import Context, Template
 from django.template.base import Node, NodeList, TextNode
 from django.template.defaulttags import CommentNode
+from django.template.loader_tags import ExtendsNode, IncludeNode, construct_relative_path
 
 
 def nodelist_has_content(nodelist: NodeList) -> bool:
@@ -20,26 +22,79 @@ class NodeTraverse(NamedTuple):
     parent: Optional["NodeTraverse"]
 
 
-def walk_nodelist(nodes: NodeList, callback: Callable[[Node], Optional[str]]) -> None:
+def walk_nodelist(
+    nodes: NodeList,
+    callback: Callable[[Node], Optional[str]],
+    context: Optional[Context] = None,
+) -> None:
     """Recursively walk a NodeList, calling `callback` for each Node."""
     node_queue: List[NodeTraverse] = [NodeTraverse(node=node, parent=None) for node in nodes]
     while len(node_queue):
         traverse = node_queue.pop()
         callback(traverse)
-        child_nodes = get_node_children(traverse.node)
+        child_nodes = get_node_children(traverse.node, context)
         child_traverses = [NodeTraverse(node=child_node, parent=traverse) for child_node in child_nodes]
         node_queue.extend(child_traverses)
 
 
-def get_node_children(node: Node) -> NodeList:
+def get_node_children(node: Node, context: Optional[Context] = None) -> NodeList:
     """
     Get child Nodes from Node's nodelist atribute.
 
     This function is taken from `get_nodes_by_type` method of `django.template.base.Node`.
     """
+    # Special case - {% extends %} tag - Load the template and go deeper
+    if isinstance(node, ExtendsNode):
+        # NOTE: When {% extends %} node is being parsed, it collects all remaining template
+        # under node.nodelist.
+        # Hence, when we come across ExtendsNode in the template, we:
+        # 1. Go over all nodes in the template using `node.nodelist`
+        # 2. Go over all nodes in the "parent" template, via `node.get_parent`
+        nodes = NodeList()
+        nodes.extend(node.nodelist)
+        template = node.get_parent(context)
+        nodes.extend(template.nodelist)
+        return nodes
+
+    # Special case - {% include %} tag - Load the template and go deeper
+    elif isinstance(node, IncludeNode):
+        template = get_template_for_include_node(node, context)
+        return template.nodelist
+
     nodes = NodeList()
     for attr in node.child_nodelists:
         nodelist = getattr(node, attr, [])
         if nodelist:
             nodes.extend(nodelist)
     return nodes
+
+
+def get_template_for_include_node(include_node: IncludeNode, context: Context) -> Template:
+    """
+    This snippet is taken directly from `IncludeNode.render()`. Unfortunately the
+    render logic doesn't separate out template loading logic from rendering, so we
+    have to copy the method.
+    """
+    template = include_node.template.resolve(context)
+    # Does this quack like a Template?
+    if not callable(getattr(template, "render", None)):
+        # If not, try the cache and select_template().
+        template_name = template or ()
+        if isinstance(template_name, str):
+            template_name = (
+                construct_relative_path(
+                    include_node.origin.template_name,
+                    template_name,
+                ),
+            )
+        else:
+            template_name = tuple(template_name)
+        cache = context.render_context.dicts[0].setdefault(include_node, {})
+        template = cache.get(template_name)
+        if template is None:
+            template = context.template.engine.select_template(template_name)
+            cache[template_name] = template
+    # Use the base.Template of a backends.django.Template.
+    elif hasattr(template, "template"):
+        template = template.template
+    return template

src/django_components/slots.py

@@ -258,15 +258,18 @@ def _try_parse_as_named_fill_tag_set(
     ComponentNodeCls: Type[Node],
 ) -> List[FillNode]:
     result = []
-    seen_name_fexps: Set[FilterExpression] = set()
+    seen_name_fexps: Set[str] = set()
     for node in nodelist:
         if isinstance(node, FillNode):
-            if node.name_fexp in seen_name_fexps:
+            # Check that, after we've resolved the names, that there's still no duplicates.
+            # This makes sure that if two different variables refer to same string, we detect
+            # them.
+            if node.name_fexp.token in seen_name_fexps:
                 raise TemplateSyntaxError(
                     f"Multiple fill tags cannot target the same slot name: "
                     f"Detected duplicate fill tag name '{node.name_fexp}'."
                 )
-            seen_name_fexps.add(node.name_fexp)
+            seen_name_fexps.add(node.name_fexp.token)
             result.append(node)
         elif isinstance(node, CommentNode):
             pass
@@ -308,6 +311,7 @@ def _try_parse_as_default_fill(
 
 
 def resolve_slots(
+    context: Context,
     template: Template,
     component_name: Optional[str],
     context_data: Dict[str, Any],
@@ -374,7 +378,7 @@ def resolve_slots(
             slot_children[parent_slot_id].append(node.node_id)
             break
 
-    walk_nodelist(template.nodelist, on_node)
+    walk_nodelist(template.nodelist, on_node, context)
 
     # 3. Figure out which slot the default/implicit fill belongs to
     slot_fills = _resolve_default_slot(

tests/templates/block_in_slot_in_component.html

@@ -0,0 +1,17 @@
+{% load component_tags %}
+<!DOCTYPE html>
+<html lang="en">
+  <body>
+    {% component "slotted_component" %}
+      {% fill "header" %}{% endfill %}
+      {% fill "main" %}
+        {% slot "body" %}
+          Helloodiddoo
+          {% block inner %}
+            Default inner
+          {% endblock %}
+        {% endslot %}
+      {% endfill %}
+    {% endcomponent %}
+  </body>
+</html>

tests/templates/block_inside_component.html

@@ -0,0 +1,13 @@
+{% load component_tags %}
+<!DOCTYPE html>
+<html lang="en">
+  <body>
+    {% component "slotted_component" %}
+      {% fill "header" %}{% endfill %}
+      {% fill "main" %}
+          {% block body %}
+          {% endblock %}
+      {% endfill %}
+    {% endcomponent %}
+  </body>
+</html>

tests/templates/extendable_template_with_blocks.html

@@ -1,12 +1,12 @@
 {% load component_tags %}
 <!DOCTYPE html>
 <html lang="en">
-<body>
+  <body>
-<main role="main">
+    <main role="main">
-  <div class='container main-container'>
+      <div class='container main-container'>
-    {% block body %}
+        {% block body %}
-    {% endblock %}
+        {% endblock %}
-  </div>
+      </div>
-</main>
+    </main>
-</body>
+  </body>
 </html>

tests/templates/slot_inside_block.html

@@ -0,0 +1,17 @@
+{% load component_tags %}
+<!DOCTYPE html>
+<html lang="en">
+  <body>
+    {% component "slotted_component" %}
+      {% fill "header" %}{% endfill %}
+      {% fill "main" %}
+        Helloodiddoo
+        {% block inner %}
+          {% slot "body" %}
+            Default inner
+          {% endslot %}
+        {% endblock %}
+      {% endfill %}
+    {% endcomponent %}
+  </body>
+</html>

tests/templates/slot_inside_extends.html

@@ -0,0 +1 @@
+{% extends "block_in_slot_in_component.html" %}

tests/templates/slot_inside_include.html

@@ -0,0 +1 @@
+{% include "block_in_slot_in_component.html" %}

tests/test_templatetags.py

@@ -114,6 +114,23 @@ class _ComplexParentComponent(component.Component):
         return {"items": items}
 
 
+class BlockInSlotInComponent(component.Component):
+    template_name = "block_in_slot_in_component.html"
+
+
+class SlotInsideExtendsComponent(component.Component):
+    template_name = "slot_inside_extends.html"
+
+
+class SlotInsideIncludeComponent(component.Component):
+    template_name = "slot_inside_include.html"
+
+
+#######################
+# TESTS
+#######################
+
+
 class ComponentTemplateTagTest(BaseTestCase):
     def setUp(self):
         # NOTE: component.registry is global, so need to clear before each test
@@ -1045,7 +1062,7 @@ class TemplateSyntaxErrorTests(BaseTestCase):
             ).render(Context({}))
 
     def test_isolated_slot_is_error(self):
-        with self.assertRaises(TemplateSyntaxError):
+        with self.assertRaises(KeyError):
             Template(
                 """
                 {% load component_tags %}
@@ -1062,13 +1079,27 @@ class TemplateSyntaxErrorTests(BaseTestCase):
             Template(
                 """
                 {% load component_tags %}
-                {% component "broken_component" %}
+                {% component "test" %}
                     {% fill "header" %}Custom header {% endfill %}
                     {% fill "header" %}Other header{% endfill %}
                 {% endcomponent %}
                 """
             ).render(Context({}))
 
+    def test_non_unique_fill_names_is_error_via_vars(self):
+        with self.assertRaises(TemplateSyntaxError):
+            Template(
+                """
+                {% load component_tags %}
+                {% with var1="header" var2="header" %}
+                    {% component "test" %}
+                        {% fill var1 %}Custom header {% endfill %}
+                        {% fill var2 %}Other header{% endfill %}
+                    {% endcomponent %}
+                {% endwith %}
+                """
+            ).render(Context({}))
+
 
 class ComponentNestingTests(BaseTestCase):
     @classmethod
@@ -1422,9 +1453,7 @@ class ContextVarsTests(BaseTestCase):
         self.assertHTMLEqual(rendered, expected)
 
 
-class RegressionTests(BaseTestCase):
+class BlockCompatTests(BaseTestCase):
-    """Ensure we don't break the same thing AGAIN."""
-
     def setUp(self):
         component.registry.clear()
         super().setUp()
@@ -1434,7 +1463,61 @@ class RegressionTests(BaseTestCase):
         super().tearDownClass()
         component.registry.clear()
 
-    def test_block_and_extends_tag_works(self):
+    def test_slots_inside_extends(self):
+        component.registry.register("slotted_component", SlottedComponent)
+        component.registry.register("slot_inside_extends", SlotInsideExtendsComponent)
+
+        template = """
+        {% load component_tags %}
+        {% component "slot_inside_extends" %}
+            {% fill "body" %}
+                BODY_FROM_FILL
+            {% endfill %}
+        {% endcomponent %}
+        """
+        rendered = Template(template).render(Context())
+        expected = """
+        <!DOCTYPE html>
+        <html lang="en">
+          <body>
+            <custom-template>
+              <header></header>
+              <main>BODY_FROM_FILL</main>
+              <footer>Default footer</footer>
+            </custom-template>
+          </body>
+        </html>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    def test_slots_inside_include(self):
+        component.registry.register("slotted_component", SlottedComponent)
+        component.registry.register("slot_inside_include", SlotInsideIncludeComponent)
+
+        template = """
+        {% load component_tags %}
+        {% component "slot_inside_include" %}
+            {% fill "body" %}
+                BODY_FROM_FILL
+            {% endfill %}
+        {% endcomponent %}
+        """
+        rendered = Template(template).render(Context())
+        expected = """
+        <!DOCTYPE html>
+        <html lang="en">
+          <body>
+            <custom-template>
+              <header></header>
+              <main>BODY_FROM_FILL</main>
+              <footer>Default footer</footer>
+            </custom-template>
+          </body>
+        </html>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    def test_component_inside_block(self):
         component.registry.register("slotted_component", SlottedComponent)
         template = """
         {% extends "extendable_template_with_blocks.html" %}
@@ -1468,6 +1551,211 @@ class RegressionTests(BaseTestCase):
         """
         self.assertHTMLEqual(rendered, expected)
 
+    def test_block_inside_component(self):
+        component.registry.register("slotted_component", SlottedComponent)
+
+        template = """
+        {% extends "block_inside_component.html" %}
+        {% load component_tags %}
+        {% block body %}
+          <div>
+            58 giraffes and 2 pantaloons
+          </div>
+        {% endblock %}
+        """
+        rendered = Template(template).render(Context())
+        expected = """
+        <!DOCTYPE html>
+        <html lang="en">
+        <body>
+            <custom-template>
+              <header></header>
+              <main>
+                <div> 58 giraffes and 2 pantaloons </div>
+              </main>
+              <footer>Default footer</footer>
+            </custom-template>
+        </body>
+        </html>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    def test_block_does_not_affect_inside_component(self):
+        """
+        Assert that when we call a component with `{% component %}`, that
+        the `{% block %}` will NOT affect the inner component.
+        """
+        component.registry.register("slotted_component", SlottedComponent)
+        component.registry.register("block_inside_slot_v1", BlockInSlotInComponent)
+
+        template = """
+        {% load component_tags %}
+        {% component "block_inside_slot_v1" %}
+            {% fill "body" %}
+                BODY_FROM_FILL
+            {% endfill %}
+        {% endcomponent %}
+        {% block inner %}
+            wow
+        {% endblock %}
+        """
+        rendered = Template(template).render(Context())
+        expected = """
+        <!DOCTYPE html>
+        <html lang="en">
+        <body>
+            <custom-template>
+              <header></header>
+              <main>BODY_FROM_FILL</main>
+              <footer>Default footer</footer>
+            </custom-template>
+        </body>
+        </html>
+        wow
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    def test_slot_inside_block__slot_default_block_default(self):
+        component.registry.register("slotted_component", SlottedComponent)
+
+        @component.register("slot_inside_block")
+        class _SlotInsideBlockComponent(component.Component):
+            template = """{% extends "slot_inside_block.html" %}"""
+
+        template = """
+            {% load component_tags %}
+            {% component "slot_inside_block" %}{% endcomponent %}
+        """
+        rendered = Template(template).render(Context())
+        expected = """
+        <!DOCTYPE html>
+        <html lang="en">
+        <body>
+            <custom-template>
+              <header></header>
+              <main>
+                Helloodiddoo
+                Default inner
+              </main>
+              <footer>Default footer</footer>
+            </custom-template>
+        </body>
+        </html>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    def test_slot_inside_block__slot_default_block_override(self):
+        component.registry.register("slotted_component", SlottedComponent)
+
+        @component.register("slot_inside_block")
+        class _SlotInsideBlockComponent(component.Component):
+            template = """
+                {% extends "slot_inside_block.html" %}
+                {% block inner %}
+                    INNER BLOCK OVERRIDEN
+                {% endblock %}
+            """
+
+        template = """
+            {% load component_tags %}
+            {% component "slot_inside_block" %}{% endcomponent %}
+        """
+        rendered = Template(template).render(Context())
+        expected = """
+        <!DOCTYPE html>
+        <html lang="en">
+        <body>
+            <custom-template>
+              <header></header>
+              <main>
+                Helloodiddoo
+                INNER BLOCK OVERRIDEN
+              </main>
+              <footer>Default footer</footer>
+            </custom-template>
+        </body>
+        </html>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    def test_slot_inside_block__slot_overriden_block_default(self):
+        component.registry.register("slotted_component", SlottedComponent)
+
+        @component.register("slot_inside_block")
+        class _SlotInsideBlockComponent(component.Component):
+            template = """{% extends "slot_inside_block.html" %}"""
+
+        template = """
+            {% load component_tags %}
+            {% component "slot_inside_block" %}
+                {% fill "body" %}
+                    SLOT OVERRIDEN
+                {% endfill %}
+            {% endcomponent %}
+        """
+        rendered = Template(template).render(Context())
+        expected = """
+        <!DOCTYPE html>
+        <html lang="en">
+        <body>
+            <custom-template>
+              <header></header>
+              <main>
+                Helloodiddoo
+                SLOT OVERRIDEN
+              </main>
+              <footer>Default footer</footer>
+            </custom-template>
+        </body>
+        </html>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    def test_slot_inside_block__slot_overriden_block_overriden(self):
+        component.registry.register("slotted_component", SlottedComponent)
+
+        @component.register("slot_inside_block")
+        class _SlotInsideBlockComponent(component.Component):
+            template = """
+                {% extends "slot_inside_block.html" %}
+                {% block inner %}
+                    {% load component_tags %}
+                    {% slot "new_slot" %}{% endslot %}
+                {% endblock %}
+                whut
+            """
+
+        # NOTE: The "body" fill will NOT show up, because we override the `inner` block
+        # with a different slot. But the "new_slot" WILL show up.
+        template = """
+            {% load component_tags %}
+            {% component "slot_inside_block" %}
+                {% fill "body" %}
+                    SLOT_BODY__OVERRIDEN
+                {% endfill %}
+                {% fill "new_slot" %}
+                    SLOT_NEW__OVERRIDEN
+                {% endfill %}
+            {% endcomponent %}
+        """
+        rendered = Template(template).render(Context())
+        expected = """
+        <!DOCTYPE html>
+        <html lang="en">
+        <body>
+            <custom-template>
+              <header></header>
+              <main>
+                Helloodiddoo
+                SLOT_NEW__OVERRIDEN
+              </main>
+              <footer>Default footer</footer>
+            </custom-template>
+        </body>
+        </html>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
 
 class IterationFillTest(BaseTestCase):
     """Tests a behaviour of {% fill .. %} tag which is inside a template {% for .. %} loop."""