fix: allow components to accept default fill even if no default slot encountered during rendering (#780)

This commit is contained in:
Juro Oravec 2024-11-26 17:42:00 +01:00 committed by GitHub
parent c0a4fd5f68
commit 9f98c7e4df
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 82 additions and 48 deletions

View file

@ -716,10 +716,6 @@ class Component(
# Get the component's HTML # Get the component's HTML
html_content = template.render(context) html_content = template.render(context)
# After we've rendered the contents, we now know what slots were there,
# and thus we can validate that.
component_slot_ctx.post_render_validation()
# Allow to optionally override/modify the rendered content # Allow to optionally override/modify the rendered content
new_output = self.on_render_after(context, template, html_content) new_output = self.on_render_after(context, template, html_content)
html_content = new_output if new_output is not None else html_content html_content = new_output if new_output is not None else html_content

View file

@ -148,41 +148,6 @@ class ComponentSlotContext:
default_slot: Optional[str] default_slot: Optional[str]
fills: Dict[SlotName, Slot] fills: Dict[SlotName, Slot]
def post_render_validation(self) -> None:
if self.is_dynamic_component:
return
default_fill = self.fills.get(DEFAULT_SLOT_KEY, None)
# Check: Only component templates that include a 'default' slot
# can be invoked with implicit filling.
if default_fill and not self.default_slot:
raise TemplateSyntaxError(
f"Component '{self.component_name}' passed default fill content "
f"(i.e. without explicit 'name' kwarg), "
f"even though none of its slots is marked as 'default'."
)
# NOTE:
# In the current implementation, the slots are resolved only at the render time.
# So when we are rendering Django's Nodes, and we come across a SlotNode, only
# at that point we check if we have the fill for it.
#
# That means that we can use variables, and we can place slots in loops.
#
# However, because the slot names are dynamic, we cannot know all the slot names
# that will be rendered ahead of the time.
#
# Moreover, user may define a slot whose default content has more slots inside it.
#
# Previously, there was an error raised if there were unfilled slots or extra fills.
#
# But now this is only a message. Because:
# 1. We don't know about ALL slots, just about the rendered ones, so we CANNOT check
# for unfilled slots (rendered slots WILL raise an error if the fill is missing).
# 2. User may provide extra fills, but these may belong to slots we haven't
# encountered in this render run. So we CANNOT say which ones are extra.
class SlotNode(BaseNode): class SlotNode(BaseNode):
"""Node corresponding to `{% slot %}`""" """Node corresponding to `{% slot %}`"""
@ -214,6 +179,27 @@ class SlotNode(BaseNode):
def __repr__(self) -> str: def __repr__(self) -> str:
return f"<Slot Node: {self.node_id}. Contents: {repr(self.nodelist)}. Options: {self.active_flags}>" return f"<Slot Node: {self.node_id}. Contents: {repr(self.nodelist)}. Options: {self.active_flags}>"
# NOTE:
# In the current implementation, the slots are resolved only at the render time.
# So when we are rendering Django's Nodes, and we come across a SlotNode, only
# at that point we check if we have the fill for it.
#
# That means that we can use variables, and we can place slots in loops.
#
# However, because the slot names are dynamic, we cannot know all the slot names
# that will be rendered ahead of the time.
#
# Moreover, user may define a `{% slot %}` whose default content has more nested
# `{% slot %}` tags inside of it.
#
# Previously, there was an error raised if there were unfilled slots or extra fills,
# or if there was an extra fill for a default slot.
#
# But we don't raise those anymore, because:
# 1. We don't know about ALL slots, just about the rendered ones, so we CANNOT check
# for unfilled slots (rendered slots WILL raise an error if the fill is missing).
# 2. User may provide extra fills, but these may belong to slots we haven't
# encountered in this render run. So we CANNOT say which ones are extra.
def render(self, context: Context) -> SafeString: def render(self, context: Context) -> SafeString:
trace_msg("RENDR", "SLOT", self.trace_id, self.node_id) trace_msg("RENDR", "SLOT", self.trace_id, self.node_id)

View file

@ -748,23 +748,75 @@ class ComponentSlotDefaultTests(BaseTestCase):
self.assertTrue(True) self.assertTrue(True)
@parametrize_context_behavior(["django", "isolated"]) @parametrize_context_behavior(["django", "isolated"])
def test_component_without_default_slot_refuses_implicit_fill(self): def test_implicit_fill_when_no_slot_marked_default(self):
registry.register("test_comp", SlottedComponent) registry.register("test_comp", SlottedComponent)
template_str: types.django_html = """ template_str: types.django_html = """
{% load component_tags %} {% load component_tags %}
{% component 'test_comp' %} {% component 'test_comp' %}
<p>This shouldn't work because the included component doesn't mark <p>Component with no 'default' slot still accepts the fill, it just won't render it</p>
any of its slots as 'default'</p> {% endcomponent %}
"""
template = Template(template_str)
rendered = template.render(Context())
self.assertHTMLEqual(
rendered,
"""
<custom-template>
<header>Default header</header>
<main>Default main</main>
<footer>Default footer</footer>
</custom-template>
""",
)
@parametrize_context_behavior(["django", "isolated"])
def test_implicit_fill_when_slot_marked_default_not_rendered(self):
@register("test_comp")
class ConditionalSlotted(Component):
def get_context_data(self, var: bool) -> Any:
return {"var": var}
template: types.django_html = """
{% load component_tags %}
<custom-template>
{% if var %}
<header>{% slot "header" default %}Default header{% endslot %}</header>
{% endif %}
<main>{% slot "main" %}Default main{% endslot %}</main>
<footer>{% slot "footer" %}Default footer{% endslot %}</footer>
</custom-template>
"""
template_str: types.django_html = """
{% load component_tags %}
{% component 'test_comp' var=var %}
123
{% endcomponent %} {% endcomponent %}
""" """
template = Template(template_str) template = Template(template_str)
with self.assertRaisesMessage( rendered_truthy = template.render(Context({"var": True}))
TemplateSyntaxError, self.assertHTMLEqual(
"Component 'test_comp' passed default fill content (i.e. without explicit 'name' kwarg), " rendered_truthy,
"even though none of its slots is marked as 'default'", """
): <custom-template>
template.render(Context()) <header>123</header>
<main>Default main</main>
<footer>Default footer</footer>
</custom-template>
""",
)
rendered_falsy = template.render(Context({"var": False}))
self.assertHTMLEqual(
rendered_falsy,
"""
<custom-template>
<main>Default main</main>
<footer>Default footer</footer>
</custom-template>
""",
)
class PassthroughSlotsTest(BaseTestCase): class PassthroughSlotsTest(BaseTestCase):