refactor: fix for nested slots (#698) (#699)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

src/django_components/component.py

@@ -40,7 +40,6 @@ from django_components.component_registry import ComponentRegistry
 from django_components.component_registry import registry as registry_
 from django_components.context import (
     _FILLED_SLOTS_CONTENT_CONTEXT_KEY,
-    _PARENT_COMP_CONTEXT_KEY,
     _REGISTRY_CONTEXT_KEY,
     _ROOT_CTX_CONTEXT_KEY,
     get_injected_context_var,
@@ -630,16 +629,10 @@ class Component(Generic[ArgsType, KwargsType, DataType, SlotsType], metaclass=Co
             else:
                 fill_content = self.fill_content
 
-            # If this is top-level component and it has no parent, use outer context instead
-            slot_context_data = context_data
-            if not context[_PARENT_COMP_CONTEXT_KEY]:
-                slot_context_data = self.outer_context.flatten()
-
             _, resolved_fills = resolve_slots(
                 context,
                 template,
                 component_name=self.name,
-                context_data=slot_context_data,
                 fill_content=fill_content,
                 # Dynamic component has a special mark do it doesn't raise certain errors
                 is_dynamic_component=getattr(self, "_is_dynamic_component", False),

src/django_components/context.py

@@ -15,7 +15,6 @@ from django_components.utils import find_last_index
 _FILLED_SLOTS_CONTENT_CONTEXT_KEY = "_DJANGO_COMPONENTS_FILLED_SLOTS"
 _ROOT_CTX_CONTEXT_KEY = "_DJANGO_COMPONENTS_ROOT_CTX"
 _REGISTRY_CONTEXT_KEY = "_DJANGO_COMPONENTS_REGISTRY"
-_PARENT_COMP_CONTEXT_KEY = "_DJANGO_COMPONENTS_PARENT_COMP"
 _CURRENT_COMP_CONTEXT_KEY = "_DJANGO_COMPONENTS_CURRENT_COMP"
 _INJECT_CONTEXT_KEY_PREFIX = "_DJANGO_COMPONENTS_INJECT__"
 
@@ -57,9 +56,6 @@ def set_component_id(context: Context, component_id: str) -> None:
     We use the Context object to pass down info on inside of which component
     we are currently rendering.
     """
-    # Store the previous component so we can detect if the current component
-    # is the top-most or not. If it is, then "_parent_component_id" is None
-    context[_PARENT_COMP_CONTEXT_KEY] = context.get(_CURRENT_COMP_CONTEXT_KEY, None)
     context[_CURRENT_COMP_CONTEXT_KEY] = component_id
 
 

src/django_components/slots.py

@@ -124,7 +124,6 @@ class SlotFill(Generic[TSlotData]):
     escaped_name: str
     is_filled: bool
     content_func: SlotFunc[TSlotData]
-    context_data: Mapping
     slot_default_var: Optional[SlotDefaultName]
     slot_data_var: Optional[SlotDataName]
 
@@ -479,7 +478,6 @@ def resolve_slots(
     context: Context,
     template: Template,
     component_name: Optional[str],
-    context_data: Mapping[str, Any],
     fill_content: Dict[SlotName, FillContent],
     is_dynamic_component: bool = False,
 ) -> Tuple[Dict[SlotId, Slot], Dict[SlotId, SlotFill]]:
@@ -497,7 +495,6 @@ def resolve_slots(
             escaped_name=_escape_slot_name(name),
             is_filled=True,
             content_func=fill.content_func,
-            context_data=context_data,
             slot_default_var=fill.slot_default_var,
             slot_data_var=fill.slot_data_var,
         )
@@ -507,6 +504,7 @@ def resolve_slots(
     slots: Dict[SlotId, Slot] = {}
     # This holds info on which slot (key) has which slots nested in it (value list)
     slot_children: Dict[SlotId, List[SlotId]] = {}
+    all_nested_slots: Set[SlotId] = set()
 
     def on_node(entry: NodeTraverse) -> None:
         node = entry.node
@@ -535,16 +533,17 @@ def resolve_slots(
         # - 0002: []
         # - 0003: [0004]
         # In other words, the data tells us that slot ID 0001 is PARENT of slot 0002.
-        curr_entry = entry.parent
-        while curr_entry and curr_entry.parent is not None:
-            if not isinstance(curr_entry.node, SlotNode):
-                curr_entry = curr_entry.parent
+        parent_slot_entry = entry.parent
+        while parent_slot_entry is not None:
+            if not isinstance(parent_slot_entry.node, SlotNode):
+                parent_slot_entry = parent_slot_entry.parent
                 continue
 
-            parent_slot_id = curr_entry.node.node_id
+            parent_slot_id = parent_slot_entry.node.node_id
             if parent_slot_id not in slot_children:
                 slot_children[parent_slot_id] = []
             slot_children[parent_slot_id].append(node.node_id)
+            all_nested_slots.add(node.node_id)
             break
 
     walk_nodelist(template.nodelist, on_node, context)
@@ -565,10 +564,7 @@ def resolve_slots(
         _report_slot_errors(slots, slot_fills, component_name)
 
     # 5. Find roots of the slot relationships
-    top_level_slot_ids: List[SlotId] = []
-    for node_id, slot in slots.items():
-        if node_id not in slot_children or not slot_children[node_id]:
-            top_level_slot_ids.append(node_id)
+    top_level_slot_ids: List[SlotId] = [node_id for node_id in slots.keys() if node_id not in all_nested_slots]
 
     # 6. Walk from out-most slots inwards, and decide whether and how
     # we will render each slot.
@@ -592,7 +588,6 @@ def resolve_slots(
                 escaped_name=_escape_slot_name(slot.name),
                 is_filled=False,
                 content_func=_nodelist_to_slot_render_func(slot.nodelist),
-                context_data=context_data,
                 slot_default_var=None,
                 slot_data_var=None,
             )
@@ -625,29 +620,30 @@ def _resolve_default_slot(
 
     # Check for errors
     for slot in slots.values():
-        if slot.is_default:
-            if default_slot_encountered:
-                raise TemplateSyntaxError(
-                    "Only one component slot may be marked as 'default'. "
-                    f"To fix, check template '{template_name}' "
-                    f"of component '{component_name}'."
-                )
-            default_slot_encountered = True
+        if not slot.is_default:
+            continue
 
-            # Here we've identified which slot the default/implicit fill belongs to
-            if default_fill:
-                # NOTE: We recreate new instance, passing all fields, instead of using
-                # `NamedTuple._replace`, because `_replace` is not typed.
-                named_fills[slot.name] = SlotFill(
-                    is_filled=default_fill.is_filled,
-                    content_func=default_fill.content_func,
-                    context_data=default_fill.context_data,
-                    slot_default_var=default_fill.slot_default_var,
-                    slot_data_var=default_fill.slot_data_var,
-                    # Updated fields
-                    name=slot.name,
-                    escaped_name=_escape_slot_name(slot.name),
-                )
+        if default_slot_encountered:
+            raise TemplateSyntaxError(
+                "Only one component slot may be marked as 'default'. "
+                f"To fix, check template '{template_name}' "
+                f"of component '{component_name}'."
+            )
+        default_slot_encountered = True
+
+        # Here we've identified which slot the default/implicit fill belongs to
+        if default_fill:
+            # NOTE: We recreate new instance, passing all fields, instead of using
+            # `NamedTuple._replace`, because `_replace` is not typed.
+            named_fills[slot.name] = SlotFill(
+                is_filled=default_fill.is_filled,
+                content_func=default_fill.content_func,
+                slot_default_var=default_fill.slot_default_var,
+                slot_data_var=default_fill.slot_data_var,
+                # Updated fields
+                name=slot.name,
+                escaped_name=_escape_slot_name(slot.name),
+            )
 
     # Check: Only component templates that include a 'default' slot
     # can be invoked with implicit filling.
@@ -725,7 +721,7 @@ def _escape_slot_name(name: str) -> str:
 
 
 def _nodelist_to_slot_render_func(nodelist: NodeList) -> SlotFunc:
-    def render_func(ctx: Context, kwargs: Dict[str, Any], slot_ref: SlotRef) -> SlotResult:
+    def render_func(ctx: Context, slot_data: Dict[str, Any], slot_ref: SlotRef) -> SlotResult:
         return nodelist.render(ctx)
 
     return render_func  # type: ignore[return-value]

tests/test_templatetags_slot_fill.py

@@ -522,6 +522,172 @@ class ComponentSlottedTemplateTagTest(BaseTestCase):
         )
 
 
+# See https://github.com/EmilStenstrom/django-components/issues/698
+class NestedSlotsTests(BaseTestCase):
+    class NestedSlots(Component):
+        template: types.django_html = """
+            {% load component_tags %}
+            {% slot 'wrapper' %}
+                <div>
+                    Wrapper Default
+                    {% slot 'parent1' %}
+                        <div>
+                            Parent1 Default
+                            {% slot 'child1' %}
+                                <div>
+                                    Child 1 Default
+                                </div>
+                            {% endslot %}
+                        </div>
+                    {% endslot %}
+                    {% slot 'parent2' %}
+                        <div>
+                            Parent2 Default
+                        </div>
+                    {% endslot %}
+                </div>
+            {% endslot %}
+        """
+
+    def setUp(self) -> None:
+        super().setUp()
+        registry.register("example", self.NestedSlots)
+
+    @parametrize_context_behavior(["django", "isolated"])
+    def test_empty(self):
+        template_str: types.django_html = """
+            {% load component_tags %}
+            {% component 'example' %}
+            {% endcomponent %}
+        """
+
+        rendered = Template(template_str).render(Context())
+        expected = """
+            <div>
+                Wrapper Default
+                <div>
+                    Parent1 Default
+                    <div>
+                        Child 1 Default
+                    </div>
+                </div>
+                <div>
+                    Parent2 Default
+                </div>
+            </div>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    @parametrize_context_behavior(["django", "isolated"])
+    def test_override_outer(self):
+        template_str: types.django_html = """
+            {% load component_tags %}
+            {% component 'example' %}
+                {% fill 'wrapper' %}
+                    <div>
+                        Entire Wrapper Replaced
+                    </div>
+                {% endfill %}
+            {% endcomponent %}
+        """
+
+        rendered = Template(template_str).render(Context())
+        expected = """
+            <div>
+                Entire Wrapper Replaced
+            </div>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    @parametrize_context_behavior(["django", "isolated"])
+    def test_override_middle(self):
+        template_str: types.django_html = """
+            {% load component_tags %}
+            {% component 'example' %}
+                {% fill 'parent1' %}
+                    <div>
+                        Parent1 Replaced
+                    </div>
+                {% endfill %}
+            {% endcomponent %}
+        """
+
+        rendered = Template(template_str).render(Context())
+        expected = """
+            <div>
+                Wrapper Default
+                <div>
+                    Parent1 Replaced
+                </div>
+                <div>
+                    Parent2 Default
+                </div>
+            </div>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    @parametrize_context_behavior(["django", "isolated"])
+    def test_override_inner(self):
+        template_str: types.django_html = """
+            {% load component_tags %}
+            {% component 'example' %}
+                {% fill 'child1' %}
+                    <div>
+                        Child1 Replaced
+                    </div>
+                {% endfill %}
+            {% endcomponent %}
+        """
+
+        rendered = Template(template_str).render(Context())
+        expected = """
+            <div>
+                Wrapper Default
+                <div>
+                    Parent1 Default
+                    <div>
+                        Child1 Replaced
+                    </div>
+                </div>
+                <div>
+                    Parent2 Default
+                </div>
+            </div>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+    @parametrize_context_behavior(["django", "isolated"])
+    def test_override_all(self):
+        template_str: types.django_html = """
+            {% load component_tags %}
+            {% component 'example' %}
+                {% fill 'child1' %}
+                    <div>
+                        Child1 Replaced
+                    </div>
+                {% endfill %}
+                {% fill 'parent1' %}
+                    <div>
+                        Parent1 Replaced
+                    </div>
+                {% endfill %}
+                {% fill 'wrapper' %}
+                    <div>
+                        Entire Wrapper Replaced
+                    </div>
+                {% endfill %}
+            {% endcomponent %}
+        """
+
+        rendered = Template(template_str).render(Context())
+        expected = """
+            <div>
+                Entire Wrapper Replaced
+            </div>
+        """
+        self.assertHTMLEqual(rendered, expected)
+
+
 class SlottedTemplateRegressionTests(BaseTestCase):
     @parametrize_context_behavior(["django", "isolated"])
     def test_slotted_template_that_uses_missing_variable(self):