Improve slot handling to allow nested components, conditional slots, and slot.super (Fixes #33, #34, #37)

Co-authored-by: rbeard0330 <@dul2k3BKW6m>

benchmarks/component_rendering.py

@@ -91,8 +91,7 @@ class RenderBenchmarks(SimpleTestCase):
                             "{% component 'inner_component' variable='foo' %}{% endslot %}{% endcomponent_block %}",
                             name='root')
         # Sanity tests
-        response = create_and_process_template_response(template)
-        response_content = response.content.decode('utf-8')
+        response_content = create_and_process_template_response(template)
         self.assertNotIn(CSS_DEPENDENCY_PLACEHOLDER, response_content)
         self.assertNotIn(JS_DEPENDENCY_PLACEHOLDER, response_content)
         self.assertIn('style.css', response_content)
@@ -109,8 +108,7 @@ class RenderBenchmarks(SimpleTestCase):
         from django.template.loader import get_template
 
         template = get_template('mdn_complete_page.html')
-        response = create_and_process_template_response(template, {})
-        response_content = response.content.decode('utf-8')
+        response_content = create_and_process_template_response(template, {})
         self.assertNotIn(CSS_DEPENDENCY_PLACEHOLDER, response_content)
         self.assertNotIn(JS_DEPENDENCY_PLACEHOLDER, response_content)
         self.assertIn('test.css', response_content)

django_components/component.py

@@ -1,11 +1,10 @@
 import warnings
-from copy import copy
+from copy import copy, deepcopy
 from functools import lru_cache
-from itertools import chain
 
 from django.conf import settings
 from django.forms.widgets import MediaDefiningClass
-from django.template.base import NodeList, TokenType
+from django.template.base import Node, NodeList, TokenType
 from django.template.loader import get_template
 from django.utils.safestring import mark_safe
 
@@ -14,6 +13,7 @@ from django_components.component_registry import AlreadyRegistered, ComponentReg
 
 TEMPLATE_CACHE_SIZE = getattr(settings, "COMPONENTS", {}).get('TEMPLATE_CACHE_SIZE', 128)
 
+
 class SimplifiedInterfaceMediaDefiningClass(MediaDefiningClass):
     def __new__(mcs, name, bases, attrs):
         if "Media" in attrs:
@@ -39,6 +39,7 @@ class SimplifiedInterfaceMediaDefiningClass(MediaDefiningClass):
 
         return super().__new__(mcs, name, bases, attrs)
 
+
 class Component(metaclass=SimplifiedInterfaceMediaDefiningClass):
 
     def __init__(self, component_name):
@@ -73,45 +74,54 @@ class Component(metaclass=SimplifiedInterfaceMediaDefiningClass):
 
     @staticmethod
     def is_slot_node(node):
-        return node.token.token_type == TokenType.BLOCK and node.token.split_contents()[0] == "slot"
+        return (isinstance(node, Node)
+                and node.token.token_type == TokenType.BLOCK
+                and node.token.split_contents()[0] == "slot")
 
     @lru_cache(maxsize=TEMPLATE_CACHE_SIZE)
-    def compile_instance_template(self, template_name):
-        """Use component's base template and the slots used for this instance to compile
-        a unified template for this instance."""
+    def get_processed_template(self, template_name):
+        """Retrieve the requested template and add a link to this component to each SlotNode in the template."""
 
-        component_template = get_template(template_name)
-        slots_in_template = Component.slots_in_template(component_template)
+        source_template = get_template(template_name)
 
-        defined_slot_names = set(slots_in_template.keys())
-        filled_slot_names = set(self.slots.keys())
-        unexpected_slots = filled_slot_names - defined_slot_names
-        if unexpected_slots:
+        # The template may be shared with another component (e.g., due to caching). To ensure that each
+        # SlotNode is unique between components, we have to copy the nodes in the template nodelist and
+        # any contained nodelists.
+        component_template = copy(source_template.template)
+        cloned_nodelist = [duplicate_node(node) for node in component_template.nodelist]
+        component_template.nodelist = NodeList(cloned_nodelist)
+
+        # Traverse template nodes and descendants, and give each slot node a reference to this component.
+        visited_nodes = set()
+        nodes_to_visit = list(component_template.nodelist)
+        slots_seen = set()
+        while nodes_to_visit:
+            current_node = nodes_to_visit.pop()
+            if current_node in visited_nodes:
+                continue
+            visited_nodes.add(current_node)
+            for nodelist_name in current_node.child_nodelists:
+                nodes_to_visit.extend(getattr(current_node, nodelist_name, []))
+            if self.is_slot_node(current_node):
+                slots_seen.add(current_node.name)
+                current_node.parent_component = self
+
+        # Check and warn for unknown slots
         if settings.DEBUG:
+            filled_slot_names = set(self.slots.keys())
+            unused_slots = filled_slot_names - slots_seen
+            if unused_slots:
                 warnings.warn(
-                    "Component {} was provided with unexpected slots: {}".format(
-                        self._component_name, unexpected_slots
+                    "Component {} was provided with slots that were not used in a template: {}".format(
+                        self._component_name, unused_slots
                     )
                 )
-            for unexpected_slot in unexpected_slots:
-                del self.slots[unexpected_slot]
 
-        combined_slots = dict(slots_in_template, **self.slots)
-        if combined_slots:
-            # Replace slot nodes with their nodelists, then combine into a single, flat nodelist
-            node_iterator = ([node] if not Component.is_slot_node(node) else combined_slots[node.name]
-                             for node in component_template.template.nodelist)
-
-            instance_template = copy(component_template.template)
-            instance_template.nodelist = NodeList(chain.from_iterable(node_iterator))
-        else:
-            instance_template = component_template.template
-
-        return instance_template
+        return component_template
 
     def render(self, context):
         template_name = self.template(context)
-        instance_template = self.compile_instance_template(template_name)
+        instance_template = self.get_processed_template(template_name)
         return instance_template.render(context)
 
     class Media:
@@ -122,6 +132,25 @@ class Component(metaclass=SimplifiedInterfaceMediaDefiningClass):
 # This variable represents the global component registry
 registry = ComponentRegistry()
 
+
+def duplicate_node(source_node):
+    """Perform a shallow copy of source_node and then recursively copy over each of source_node's nodelists.
+
+    If a nodelist is a dynamic property that cannot be set, fall back to a deepcopy of source_node."""
+
+    try:
+        clone = copy(source_node)
+        for nodelist_name in source_node.child_nodelists:
+            nodelist = getattr(source_node, nodelist_name, NodeList())
+            nodelist_contents = [duplicate_node(n) for n in nodelist]
+            setattr(clone, nodelist_name, type(nodelist)(nodelist_contents))
+        return clone
+    except AttributeError:
+        # AttributeError is raised if an attribute cannot be set (e.g., IfNode's nodelist,
+        # which is a read-only property).
+        return deepcopy(source_node)
+
+
 def register(name):
     """Class decorator to register a component.
 
@@ -133,6 +162,7 @@ def register(name):
         ...
 
     """
+
     def decorator(component):
         registry.register(name=name, component=component)
         return component

django_components/templatetags/component_tags.py

@@ -11,7 +11,7 @@ from django_components.middleware import CSS_DEPENDENCY_PLACEHOLDER, JS_DEPENDEN
 
 register = template.Library()
 
-COMPONENT_CONTEXT_KEY = "component_context"
+SLOT_CONTEXT_KEY = "__slot_context"
 
 
 def get_components_from_registry(registry):
@@ -79,14 +79,28 @@ def do_component(parser, token):
 class SlotNode(Node):
     def __init__(self, name, nodelist, component=None):
         self.name, self.nodelist, self.component = name, nodelist, component
+        self.component = None
+        self.parent_component = None
+        self.context = None
 
     def __repr__(self):
         return "<Slot Node: %s. Contents: %r>" % (self.name, self.nodelist)
 
     def render(self, context):
-        # This method should only be called if a slot tag is used outside of a component.
-        assert self.component is None
-        return self.nodelist.render(context)
+        # Thread safety: storing the context as a property of the cloned SlotNode without using
+        # the render_context facility should be thread-safe, since each cloned_node
+        # is only used for a single render.
+        cloned_node = SlotNode(self.name, self.nodelist, self.component)
+        cloned_node.parent_component = self.parent_component
+        cloned_node.context = context
+
+        assert not NodeList(), "Logic in SlotNode.render method assumes that empty nodelists are falsy."
+        with context.update({'slot': cloned_node}):
+            return cloned_node.parent_component.slots.get(cloned_node.name, cloned_node.nodelist).render(context)
+
+    def super(self):
+        """Render default slot content."""
+        return mark_safe(self.nodelist.render(self.context))
 
 
 @register.tag("slot")
@@ -103,19 +117,24 @@ def do_slot(parser, token, component=None):
 
 
 class ComponentNode(Node):
+    class InvalidSlot:
+        def super(self):
+            raise TemplateSyntaxError('slot.super may only be called within a {% slot %}/{% endslot %} block.')
+
     def __init__(self, component, context_args, context_kwargs, slots=None, isolated_context=False):
         self.context_args = context_args or []
         self.context_kwargs = context_kwargs or {}
         self.component, self.isolated_context = component, isolated_context
-        slot_dict = defaultdict(NodeList)
-        if slots:
-            for slot in slots:
-                slot_dict[slot.name].extend(slot.nodelist)
-        self.component.slots = slot_dict
+
+        # Group slot notes by name and concatenate their nodelists
+        self.component.slots = defaultdict(NodeList)
+        for slot in slots or []:
+            self.component.slots[slot.name].extend(slot.nodelist)
         self.should_render_dependencies = is_dependency_middleware_active()
 
     def __repr__(self):
-        return "<Component Node: %s. Contents: %r>" % (self.component, self.component.instance_template.nodelist)
+        return "<Component Node: %s. Contents: %r>" % (self.component,
+                                                       getattr(self.component.instance_template, 'nodelist', None))
 
     def render(self, context):
         self.component.outer_context = context.flatten()
@@ -123,9 +142,7 @@ class ComponentNode(Node):
         # Resolve FilterExpressions and Variables that were passed as args to the component, then call component's
         # context method to get values to insert into the context
         resolved_context_args = [safe_resolve(arg, context) for arg in self.context_args]
-        resolved_context_kwargs = {
-            key: safe_resolve(kwarg, context) for key, kwarg in self.context_kwargs.items()
-        }
+        resolved_context_kwargs = {key: safe_resolve(kwarg, context) for key, kwarg in self.context_kwargs.items()}
         component_context = self.component.context(*resolved_context_args, **resolved_context_kwargs)
 
         # Create a fresh context if requested
@@ -133,6 +150,7 @@ class ComponentNode(Node):
             context = context.new()
 
         with context.update(component_context):
+            context.render_context[SLOT_CONTEXT_KEY] = {}
             rendered_component = self.component.render(context)
             if self.should_render_dependencies:
                 return f'<!-- _RENDERED {self.component._component_name} -->' + rendered_component
@@ -158,31 +176,36 @@ def do_component_block(parser, token):
     bits = token.split_contents()
     bits, isolated_context = check_for_isolated_context_keyword(bits)
 
-    tag_name, token = next_block_token(parser)
     component, context_args, context_kwargs = parse_component_with_args(parser, bits, 'component_block')
 
-    slots_filled = NodeList()
-    while tag_name != "endcomponent_block":
-        if tag_name == "slot":
-            slots_filled += do_slot(parser, token, component=component)
-        tag_name, token = next_block_token(parser)
-
-    return ComponentNode(component, context_args, context_kwargs, slots=slots_filled,
+    return ComponentNode(component, context_args, context_kwargs,
+                         slots=[do_slot(parser, slot_token, component=component)
+                                for slot_token in slot_tokens(parser)],
                          isolated_context=isolated_context)
 
 
-def next_block_token(parser):
-    """Return tag and token for next block token.
+def slot_tokens(parser):
+    """Yield each 'slot' token appearing before the next 'endcomponent_block' token.
 
-    Raises IndexError if there are not more block tokens in the remainder of the template."""
+    Raises TemplateSyntaxError if there are other content tokens or if there is no endcomponent_block token."""
+
+    def is_whitespace(token):
+        return token.token_type == TokenType.TEXT and not token.contents.strip()
+
+    def is_block_tag(token, /, name):
+        return token.token_type == TokenType.BLOCK and token.split_contents()[0] == name
 
     while True:
+        try:
             token = parser.next_token()
-        if token.token_type != TokenType.BLOCK:
-            continue
-
-        tag_name = token.split_contents()[0]
-        return tag_name, token
+        except IndexError:
+            raise TemplateSyntaxError('Unclosed component_block tag')
+        if is_block_tag(token, name='endcomponent_block'):
+            return
+        elif is_block_tag(token, name='slot'):
+            yield token
+        elif not is_whitespace(token) and token.token_type != TokenType.COMMENT:
+            raise TemplateSyntaxError(f'Content tokens in component blocks must be inside of slot tags: {token}')
 
 
 def check_for_isolated_context_keyword(bits):

tests/templates/conditional_template.html

@@ -0,0 +1,6 @@
+{% load component_tags %}
+{% if branch == 'a' %}
+    <p id="a">{% slot 'a' %}Default A{% endslot %}</p>
+{% elif branch == 'b' %}
+    <p id="b">{% slot 'b' %}Default B{% endslot %}</p>
+{% endif %}

tests/templates/nested_slot_template.html

@@ -0,0 +1,4 @@
+{% load component_tags %}
+{% slot 'outer' %}
+    <div id="outer">{% slot 'inner' %}Default{% endslot %}</div>
+{% endslot %}

tests/test_component.py

@@ -1,11 +1,11 @@
 from textwrap import dedent
 
-from django.template import Context
+from django.template import Context, Template
 
 from .django_test_setup import *  # NOQA
-from .testutils import Django30CompatibleSimpleTestCase as SimpleTestCase
 
 from django_components import component
+from .testutils import Django30CompatibleSimpleTestCase as SimpleTestCase
 
 
 class ComponentTest(SimpleTestCase):
@@ -98,6 +98,8 @@ class ComponentTest(SimpleTestCase):
             """)
         )
 
+
+class ComponentMediaTests(SimpleTestCase):
     def test_component_media_with_strings(self):
         class SimpleComponent(component.Component):
             class Media:
@@ -164,3 +166,52 @@ class ComponentTest(SimpleTestCase):
                 <script src="path/to/script.js"></script>
             """)
         )
+
+
+class ComponentIsolationTests(SimpleTestCase):
+    def setUp(self):
+        class SlottedComponent(component.Component):
+            def template(self, context):
+                return "slotted_template.html"
+
+        component.registry.register('test', SlottedComponent)
+
+    def test_instances_of_component_do_not_share_slots(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block "test" %}
+                {% slot "header" %}Override header{% endslot %}
+            {% endcomponent_block %}
+            {% component_block "test" %}
+                {% slot "main" %}Override main{% endslot %}
+            {% endcomponent_block %}
+            {% component_block "test" %}
+                {% slot "footer" %}Override footer{% endslot %}
+            {% endcomponent_block %}
+        """
+        )
+
+        template.render(Context({}))
+        rendered = template.render(Context({}))
+
+        self.assertHTMLEqual(
+            rendered,
+            """
+            <custom-template>
+                <header>Override header</header>
+                <main>Default main</main>
+                <footer>Default footer</footer>
+            </custom-template>
+            <custom-template>
+                <header>Default header</header>
+                <main>Override main</main>
+                <footer>Default footer</footer>
+            </custom-template>
+            <custom-template>
+                <header>Default header</header>
+                <main>Default main</main>
+                <footer>Override footer</footer>
+            </custom-template>
+        """
+        )

tests/test_templatetags.py

@@ -1,6 +1,6 @@
 from textwrap import dedent
 
-from django.template import Context, Template
+from django.template import Context, Template, TemplateSyntaxError
 
 from .django_test_setup import *  # NOQA
 from django_components import component
@@ -356,3 +356,281 @@ class TemplateInstrumentationTest(SimpleTestCase):
         templates_used = self.templates_used_to_render(template)
         self.assertIn('slotted_template.html', templates_used)
         self.assertIn('simple_template.html', templates_used)
+
+
+class NestedSlotTests(SimpleTestCase):
+    class NestedComponent(component.Component):
+        def context(self):
+            return {}
+
+        def template(self, context):
+            return "nested_slot_template.html"
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.clear()
+        component.registry.register('test', cls.NestedComponent)
+
+    @classmethod
+    def tearDownClass(cls):
+        super().tearDownClass()
+        component.registry.clear()
+
+    def test_default_slots_render_correctly(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block 'test' %}{% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({}))
+        self.assertHTMLEqual(rendered, '<div id="outer">Default</div>')
+
+    def test_inner_slot_overriden(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block 'test' %}{% slot 'inner' %}Override{% endslot %}{% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({}))
+        self.assertHTMLEqual(rendered, '<div id="outer">Override</div>')
+
+    def test_outer_slot_overriden(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block 'test' %}{% slot 'outer' %}<p>Override</p>{% endslot %}{% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({}))
+        self.assertHTMLEqual(rendered, '<p>Override</p>')
+
+    def test_both_overriden_and_inner_removed(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block 'test' %}
+                {% slot 'outer' %}<p>Override</p>{% endslot %}
+                {% slot 'inner' %}<p>Will not appear</p>{% endslot %}
+            {% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({}))
+        self.assertHTMLEqual(rendered, '<p>Override</p>')
+
+
+class ConditionalSlotTests(SimpleTestCase):
+    class ConditionalComponent(component.Component):
+        def context(self, branch=None):
+            return {'branch': branch}
+
+        def template(self, context):
+            return "conditional_template.html"
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.clear()
+        component.registry.register('test', cls.ConditionalComponent)
+
+    @classmethod
+    def tearDownClass(cls):
+        super().tearDownClass()
+        component.registry.clear()
+
+    def test_no_content_if_branches_are_false(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block 'test' %}
+                {% slot 'a' %}Override A{% endslot %}
+                {% slot 'b' %}Override B{% endslot %}
+            {% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({}))
+        self.assertHTMLEqual(rendered, '')
+
+    def test_default_content_if_no_slots(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component 'test' branch='a' %}
+            {% component 'test' branch='b' %}
+        """
+        )
+        rendered = template.render(Context({}))
+        self.assertHTMLEqual(rendered, '<p id="a">Default A</p><p id="b">Default B</p>')
+
+    def test_one_slot_overridden(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block 'test' branch='a' %}
+                {% slot 'b' %}Override B{% endslot %}
+            {% endcomponent_block %}
+            {% component_block 'test' branch='b' %}
+                {% slot 'b' %}Override B{% endslot %}
+            {% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({}))
+        self.assertHTMLEqual(rendered, '<p id="a">Default A</p><p id="b">Override B</p>')
+
+    def test_both_slots_overridden(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block 'test' branch='a' %}
+                {% slot 'a' %}Override A{% endslot %}
+                {% slot 'b' %}Override B{% endslot %}
+            {% endcomponent_block %}
+            {% component_block 'test' branch='b' %}
+                {% slot 'a' %}Override A{% endslot %}
+                {% slot 'b' %}Override B{% endslot %}
+            {% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({}))
+        self.assertHTMLEqual(rendered, '<p id="a">Override A</p><p id="b">Override B</p>')
+
+
+class SlotSuperTests(SimpleTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.clear()
+        component.registry.register('test', SlottedComponent)
+
+    @classmethod
+    def tearDownClass(cls):
+        super().tearDownClass()
+        component.registry.clear()
+
+    def test_basic_super_functionality(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block "test" %}
+                {% slot "header" %}Before: {{ slot.super }}{% endslot %}
+                {% slot "main" %}{{ slot.super }}{% endslot %}
+                {% slot "footer" %}{{ slot.super }}, after{% endslot %}
+            {% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({}))
+
+        self.assertHTMLEqual(
+            rendered,
+            """
+            <custom-template>
+                <header>Before: Default header</header>
+                <main>Default main</main>
+                <footer>Default footer, after</footer>
+            </custom-template>
+        """,
+        )
+
+    def test_multiple_super_calls(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block "test" %}
+                {% slot "header" %}First: {{ slot.super }}; Second: {{ slot.super }}{% endslot %}
+            {% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({}))
+
+        self.assertHTMLEqual(
+            rendered,
+            """
+            <custom-template>
+                <header>First: Default header; Second: Default header</header>
+                <main>Default main</main>
+                <footer>Default footer</footer>
+            </custom-template>
+        """,
+        )
+
+    def test_super_under_if_node(self):
+        template = Template(
+            """
+            {% load component_tags %}
+            {% component_block "test" %}
+                {% slot "header" %}
+                    {% for i in range %}
+                        {% if forloop.first %}First {{slot.super}}
+                        {% else %}Later {{ slot.super }}
+                        {% endif %}
+                    {%endfor %}
+                {% endslot %}
+            {% endcomponent_block %}
+        """
+        )
+        rendered = template.render(Context({'range': range(3)}))
+
+        self.assertHTMLEqual(
+            rendered,
+            """
+            <custom-template>
+                <header>First Default header Later Default header Later Default header</header>
+                <main>Default main</main>
+                <footer>Default footer</footer>
+            </custom-template>
+        """,
+        )
+
+
+class TemplateSyntaxErrorTests(SimpleTestCase):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        component.registry.register('test', SlottedComponent)
+
+    def test_variable_outside_slot_tag_is_error(self):
+        with self.assertRaises(TemplateSyntaxError):
+            Template(
+                """
+                {% load component_tags %}
+                {% component_block "test" %}
+                    {{ slot.super }}
+                {% endcomponent_block %}
+            """
+            )
+
+    def test_text_outside_slot_tag_is_error(self):
+        with self.assertRaises(TemplateSyntaxError):
+            Template(
+                """
+                {% load component_tags %}
+                {% component_block "test" %}
+                    Text
+                {% endcomponent_block %}
+            """
+            )
+
+    def test_nonslot_block_outside_slot_tag_is_error(self):
+        with self.assertRaises(TemplateSyntaxError):
+            Template(
+                """
+                {% load component_tags %}
+                {% component_block "test" %}
+                    {% if True %}
+                        {% slot "header" %}{% endslot %}
+                    {% endif %}
+                {% endcomponent_block %}
+            """
+            )
+
+    def test_unclosed_component_block_is_error(self):
+        with self.assertRaises(TemplateSyntaxError):
+            Template(
+                """
+                {% load component_tags %}
+                {% component_block "test" %}
+                    {% slot "header" %}{% endslot %}
+            """
+            )