From dd6d668b5e94ac0f511bd63c2c8cad96e56635fc Mon Sep 17 00:00:00 2001 From: Juro Oravec Date: Wed, 27 Nov 2024 17:19:41 +0100 Subject: [PATCH] fix: do not render slot tag when extracting fill tags (#786) * fix: do not render slot tag when extracting fill tags * refactor: show component path in error message * docs: update changelog * refactor: fix tests --- CHANGELOG.md | 26 ++++++++++++ src/django_components/component.py | 26 +++++++++++- src/django_components/slots.py | 6 +++ tests/test_component.py | 61 +++++++++++++++++++++++++++ tests/test_templatetags_component.py | 2 +- tests/test_templatetags_provide.py | 63 ++++++++++++++++++++++++++++ 6 files changed, 182 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c2a8ab0..c61a5a2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/django_components/component.py b/src/django_components/component.py index cbccfa68..f650c968 100644 --- a/src/django_components/component.py +++ b/src/django_components/component.py @@ -617,7 +617,31 @@ class Component( 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, diff --git a/src/django_components/slots.py b/src/django_components/slots.py index 7464ca9a..d021c78d 100644 --- a/src/django_components/slots.py +++ b/src/django_components/slots.py @@ -203,6 +203,12 @@ class SlotNode(BaseNode): 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. " diff --git a/tests/test_component.py b/tests/test_component.py index e28d3c50..4446d200 100644 --- a/tests/test_component.py +++ b/tests/test_component.py @@ -318,6 +318,67 @@ class ComponentTest(BaseTestCase): """, ) + @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 %} +
injected: {{ data|safe }}
+
+ {% slot "content" default / %} +
+ """ + + 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): diff --git a/tests/test_templatetags_component.py b/tests/test_templatetags_component.py index 5d6c92ed..2a6132ce 100644 --- a/tests/test_templatetags_component.py +++ b/tests/test_templatetags_component.py @@ -483,7 +483,7 @@ class DynamicComponentTemplateTagTest(BaseTestCase): """ 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({})) diff --git a/tests/test_templatetags_provide.py b/tests/test_templatetags_provide.py index 6e95765c..957b03e9 100644 --- a/tests/test_templatetags_provide.py +++ b/tests/test_templatetags_provide.py @@ -740,6 +740,7 @@ class InjectTest(BaseTestCase): 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") @@ -804,3 +805,65 @@ class InjectTest(BaseTestCase):
456
""", ) + + # 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 %} +
injected: {{ data|safe }}
+
+ {% slot "content" default / %} +
+ """ + + 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, + """ +
+ injected: DepInject(key='hi', data=123) +
+
+ """, + )