Rework slot management to avoid nodelist copying (fixes #64)

Co-authored-by: rbeard0330 <@dul2k3BKW6m>

django_components/component.py

@@ -1,10 +1,9 @@
 import warnings
-from copy import copy, deepcopy
 from functools import lru_cache
 
 from django.conf import settings
 from django.forms.widgets import MediaDefiningClass
-from django.template.base import Node, NodeList, TokenType
+from django.template.base import Node, TokenType
 from django.template.loader import get_template
 from django.utils.safestring import mark_safe
 
@@ -12,6 +11,7 @@ from django.utils.safestring import mark_safe
 from django_components.component_registry import AlreadyRegistered, ComponentRegistry, NotRegistered  # noqa
 
 TEMPLATE_CACHE_SIZE = getattr(settings, "COMPONENTS", {}).get('TEMPLATE_CACHE_SIZE', 128)
+ACTIVE_SLOT_CONTEXT_KEY = '_DJANGO_COMPONENTS_ACTIVE_SLOTS'
 
 
 class SimplifiedInterfaceMediaDefiningClass(MediaDefiningClass):
@@ -80,18 +80,11 @@ class Component(metaclass=SimplifiedInterfaceMediaDefiningClass):
 
     @lru_cache(maxsize=TEMPLATE_CACHE_SIZE)
     def get_processed_template(self, template_name):
-        """Retrieve the requested template and add a link to this component to each SlotNode in the template."""
+        """Retrieve the requested template and check for unused slots."""
 
-        source_template = get_template(template_name)
+        component_template = get_template(template_name).template
 
-        # The template may be shared with another component (e.g., due to caching). To ensure that each
+        # Traverse template nodes and descendants
-        # 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()
@@ -104,7 +97,6 @@ class Component(metaclass=SimplifiedInterfaceMediaDefiningClass):
                 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:
@@ -122,7 +114,9 @@ class Component(metaclass=SimplifiedInterfaceMediaDefiningClass):
     def render(self, context):
         template_name = self.template(context)
         instance_template = self.get_processed_template(template_name)
-        return instance_template.render(context)
+        active_slots = {**context.get(ACTIVE_SLOT_CONTEXT_KEY, {}), **self.slots}
+        with context.update({ACTIVE_SLOT_CONTEXT_KEY: active_slots}):
+            return instance_template.render(context)
 
     class Media:
         css = {}
@@ -133,24 +127,6 @@ class Component(metaclass=SimplifiedInterfaceMediaDefiningClass):
 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.
 

django_components/templatetags/component_tags.py

@@ -1,3 +1,4 @@
+import inspect
 from collections import defaultdict
 
 from django import template
@@ -6,13 +7,11 @@ from django.template.base import Node, NodeList, TemplateSyntaxError, TokenType
 from django.template.library import parse_bits
 from django.utils.safestring import mark_safe
 
-from django_components.component import registry
+from django_components.component import ACTIVE_SLOT_CONTEXT_KEY, Component, registry
 from django_components.middleware import CSS_DEPENDENCY_PLACEHOLDER, JS_DEPENDENCY_PLACEHOLDER
 
 register = template.Library()
 
-SLOT_CONTEXT_KEY = "__slot_context"
-
 
 def get_components_from_registry(registry):
     """Returns a list unique components from the registry."""
@@ -94,9 +93,17 @@ class SlotNode(Node):
         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)
+            return self.get_nodelist(context).render(context)
+
+    def get_nodelist(self, context):
+        if ACTIVE_SLOT_CONTEXT_KEY not in context:
+            raise TemplateSyntaxError(f'Attempted to render SlotNode {self.name} outside of a parent Component or '
+                                      'without access to context provided by its parent Component. This will not'
+                                      'work properly.')
+
+        overriding_nodelist = context[ACTIVE_SLOT_CONTEXT_KEY].get(self.name, None)
+        return overriding_nodelist if overriding_nodelist is not None else self.nodelist
 
     def super(self):
         """Render default slot content."""
@@ -150,7 +157,6 @@ 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
@@ -192,7 +198,7 @@ def slot_tokens(parser):
     def is_whitespace(token):
         return token.token_type == TokenType.TEXT and not token.contents.strip()
 
-    def is_block_tag(token, /, name):
+    def is_block_tag(token, name):
         return token.token_type == TokenType.BLOCK and token.split_contents()[0] == name
 
     while True:

tests/templates/template_with_illegal_slot.html

@@ -0,0 +1,2 @@
+{% load component_tags %}
+{% include 'slotted_template.html' with context=None only %}

tests/test_templatetags.py

@@ -2,9 +2,9 @@ from textwrap import dedent
 
 from django.template import Context, Template, TemplateSyntaxError
 
-from .django_test_setup import *  # NOQA
 from django_components import component
 
+from .django_test_setup import *  # NOQA
 from .testutils import Django30CompatibleSimpleTestCase as SimpleTestCase
 
 
@@ -33,6 +33,11 @@ class SlottedComponent(component.Component):
         return "slotted_template.html"
 
 
+class BrokenComponent(component.Component):
+    def template(self, context):
+        return "template_with_illegal_slot.html"
+
+
 class SlottedComponentWithMissingVariable(component.Component):
     def template(self, context):
         return "slotted_template_with_missing_variable.html"
@@ -589,6 +594,7 @@ class TemplateSyntaxErrorTests(SimpleTestCase):
     def setUpClass(cls):
         super().setUpClass()
         component.registry.register('test', SlottedComponent)
+        component.registry.register('broken_component', BrokenComponent)
 
     def test_variable_outside_slot_tag_is_error(self):
         with self.assertRaises(TemplateSyntaxError):
@@ -634,3 +640,25 @@ class TemplateSyntaxErrorTests(SimpleTestCase):
                     {% slot "header" %}{% endslot %}
             """
             )
+
+    def test_slot_with_no_parent_is_error(self):
+        with self.assertRaises(TemplateSyntaxError):
+            Template(
+                """
+                {% load component_tags %}
+                {% slot "header" %}contents{% endslot %}
+            """
+            ).render(Context({}))
+
+    def test_isolated_slot_is_error(self):
+        with self.assertRaises(TemplateSyntaxError):
+            Template(
+                """
+                {% load component_tags %}
+                {% component_block "broken_component" %}
+                    {% slot "header" %}Custom header{% endslot %}
+                    {% slot "main" %}Custom main{% endslot %}
+                    {% slot "footer" %}Custom footer{% endslot %}
+                {% endcomponent_block %}
+            """
+            ).render(Context({}))