Skip to content

Commit

Permalink
fix: do not render slot tag when extracting fill tags (#786)
Browse files Browse the repository at this point in the history
* fix: do not render slot tag when extracting fill tags

* refactor: show component path in error message

* docs: update changelog

* refactor: fix tests
  • Loading branch information
JuroOravec authored Nov 27, 2024
1 parent 5a23101 commit dd6d668
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 2 deletions.
26 changes: 26 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,31 @@
# Release notes

## v0.114

#### Fix

- Prevent rendering Slot tags during fill discovery stage to fix a case when a component inside a slot
fill tried to access provided data too early.

## v0.113

#### Fix

- Ensure consistent order of scripts in `Component.Media.js`

## v0.112

#### Fix

- Allow components to accept default fill even if no default slot was encountered during rendering

## v0.111

#### Fix

- Prevent rendering Component tags during fill discovery stage to fix a case when a component inside the default slot
tried to access provided data too early.

## 🚨📢 v0.110

### General
Expand Down
26 changes: 25 additions & 1 deletion src/django_components/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,31 @@ def _render(
try:
return self._render_impl(context, args, kwargs, slots, escape_slots_content, type, render_dependencies)
except Exception as err:
raise _type(err)(f"An error occured while rendering component '{self.name}':\n{repr(err)}") from err
# Nicely format the error message to include the component path.
# E.g.
# ```
# KeyError: "An error occured while rendering components ProjectPage > ProjectLayoutTabbed >
# Layout > RenderContextProvider > Base > TabItem:
# Component 'TabItem' tried to inject a variable '_tab' before it was provided.
# ```

if not hasattr(err, "_components"):
err._components = [] # type: ignore[attr-defined]

components = getattr(err, "_components", [])

# Access the exception's message, see https://stackoverflow.com/a/75549200/9788634
if not components:
orig_msg = err.args[0]
else:
orig_msg = err.args[0].split("\n", 1)[1]

components.insert(0, self.name)
comp_path = " > ".join(components)
prefix = f"An error occured while rendering components {comp_path}:\n"

err.args = (prefix + orig_msg,) # tuple of one
raise err

def _render_impl(
self,
Expand Down
6 changes: 6 additions & 0 deletions src/django_components/slots.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,12 @@ def __repr__(self) -> str:
def render(self, context: Context) -> SafeString:
trace_msg("RENDR", "SLOT", self.trace_id, self.node_id)

# Do not render `{% slot %}` tags within the `{% component %} .. {% endcomponent %}` tags
# at the fill discovery stage (when we render the component's body to determine if the body
# is a default slot, or contains named slots).
if _is_extracting_fill(context):
return ""

if _COMPONENT_SLOT_CTX_CONTEXT_KEY not in context or not context[_COMPONENT_SLOT_CTX_CONTEXT_KEY]:
raise TemplateSyntaxError(
"Encountered a SlotNode outside of a ComponentNode context. "
Expand Down
61 changes: 61 additions & 0 deletions tests/test_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,67 @@ def get_template(self, context):
""",
)

@parametrize_context_behavior(["django", "isolated"])
def test_prepends_exceptions_with_component_path(self):
@register("broken")
class Broken(Component):
template: types.django_html = """
{% load component_tags %}
<div> injected: {{ data|safe }} </div>
<main>
{% slot "content" default / %}
</main>
"""

def get_context_data(self):
data = self.inject("my_provide")
data["data1"] # This should raise TypeError
return {"data": data}

@register("provider")
class Provider(Component):
def get_context_data(self, data: Any) -> Any:
return {"data": data}

template: types.django_html = """
{% load component_tags %}
{% provide "my_provide" key="hi" data=data %}
{% slot "content" default / %}
{% endprovide %}
"""

@register("parent")
class Parent(Component):
def get_context_data(self, data: Any) -> Any:
return {"data": data}

template: types.django_html = """
{% load component_tags %}
{% component "provider" data=data %}
{% component "broken" %}
{% slot "content" default / %}
{% endcomponent %}
{% endcomponent %}
"""

@register("root")
class Root(Component):
template: types.django_html = """
{% load component_tags %}
{% component "parent" data=123 %}
{% fill "content" %}
456
{% endfill %}
{% endcomponent %}
"""

with self.assertRaisesMessage(
TypeError,
"An error occured while rendering components Root > parent > provider > broken:\n"
"tuple indices must be integers or slices, not str",
):
Root.render()


class ComponentValidationTest(BaseTestCase):
def test_validate_input_passes(self):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_templatetags_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ def test_raises_on_invalid_args(self):
"""

template = Template(simple_tag_template)
with self.assertRaisesMessage(TypeError, "got an unexpected keyword argument \\'invalid_variable\\'"):
with self.assertRaisesMessage(TypeError, "got an unexpected keyword argument 'invalid_variable'"):
template.render(Context({}))


Expand Down
63 changes: 63 additions & 0 deletions tests/test_templatetags_provide.py
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,7 @@ def get_context_data(self):
with self.assertRaises(RuntimeError):
comp.inject("abc", "def")

# See https://github.com/EmilStenstrom/django-components/pull/778
@parametrize_context_behavior(["django", "isolated"])
def test_inject_in_fill(self):
@register("injectee")
Expand Down Expand Up @@ -804,3 +805,65 @@ class Root(Component):
<main>456</main>
""",
)

# See https://github.com/EmilStenstrom/django-components/pull/786
@parametrize_context_behavior(["django", "isolated"])
def test_inject_in_slot_in_fill(self):
@register("injectee")
class Injectee(Component):
template: types.django_html = """
{% load component_tags %}
<div> injected: {{ data|safe }} </div>
<main>
{% slot "content" default / %}
</main>
"""

def get_context_data(self):
data = self.inject("my_provide")
return {"data": data}

@register("provider")
class Provider(Component):
def get_context_data(self, data: Any) -> Any:
return {"data": data}

template: types.django_html = """
{% load component_tags %}
{% provide "my_provide" key="hi" data=data %}
{% slot "content" default / %}
{% endprovide %}
"""

@register("parent")
class Parent(Component):
def get_context_data(self, data: Any) -> Any:
return {"data": data}

template: types.django_html = """
{% load component_tags %}
{% component "provider" data=data %}
{% slot "content" default / %}
{% endcomponent %}
"""

@register("root")
class Root(Component):
template: types.django_html = """
{% load component_tags %}
{% component "parent" data=123 %}
{% component "injectee" / %}
{% endcomponent %}
"""

rendered = Root.render()

self.assertHTMLEqual(
rendered,
"""
<div>
injected: DepInject(key='hi', data=123)
</div>
<main></main>
""",
)

0 comments on commit dd6d668

Please sign in to comment.