Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: loader tags compatibility #468

Conversation

JuroOravec
Copy link
Collaborator

Added tests to explore behavior of components with "loader tags" (extends, include, block). Tested out all combinations I could think of:

  • Slots inside include
  • Slots inside extends
  • Slots inside block
  • Block inside slot

Surprisingly, it all mostly worked as expected 😄, and I had to update only the traversal logic, so it could extract nodes from extends / include tags.

Closes #134

# 2. Go over all nodes in the "parent" template, via `node.get_parent`
nodes = NodeList()
nodes.extend(node.nodelist)
template = node.get_parent(context)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To resolve the parent Template referenced via {% extends %} tag, we needed to pass it a Context. That's why much of the non-test changes is adding a context argument to functions

This snippet is taken directly from `IncludeNode.render()`. Unfortunately the
render logic doesn't separate out template loading logic from rendering, so we
have to copy the method.
"""
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As it says above, this function is a copy from Django's codebase

@@ -1062,7 +1079,7 @@ def test_non_unique_fill_names_is_error(self):
Template(
"""
{% load component_tags %}
{% component "broken_component" %}
{% component "test" %}
Copy link
Collaborator Author

@JuroOravec JuroOravec May 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was using a wrong component for testing duplicate fills. After I fixed it, it turned out there were a few issues with duplicate fill detection.

Copy link
Owner

@EmilStenstrom EmilStenstrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK with releasing this, even though I don't understand it fully.

What would you say is the intuition on how block and slot works together? That the outermost tag executes first?

@JuroOravec
Copy link
Collaborator Author

I'll write the behavior out, since it also took me some time to wrap my head around it:

  1. First let's clarify how include and extends tags work inside components. So when component template includes include or extends tags, it's as if the "included" template was inlined. So if the "included" template contains slot tags, then the component uses those slots.

    So if I have a template abc.html:

    <div>
      hello
      {% slot "body" %}{% endslot %}
    </div>

    And components that make use of abc.html via include or extends:

    @component.register("my_comp_extends")
    class MyCompWithExtends(component.Component):
        template = """{% extends "abc.html" %}"""
    
    @component.register("my_comp_include")
    class MyCompWithInclude(component.Component):
        template = """{% include "abc.html" %}"""

    Then I can set slot fill for the slot imported via include/extends:

    {% component "my_comp_extends" %}
    	{% fill "body" %}
    		123
    	{% endfill %}
    {% endcomponent %}

    And it will render:

    <div>
      hello
      123
    </div>
  2. Slot and block

    So if I have a template abc.html like so:

    <div>
      hello
      {% block inner %}
        1
        {% slot "body" %}
          2
        {% endslot %}
      {% endblock %}
    </div>

    and component my_comp:

    @component.register("my_comp")
    class MyComp(component.Component):
    	template_name = "abc.html"

    Then:

    1. Since the block wasn't overriden, we can use the body slot:

      {% component "my_comp" %}
      	{% fill "body" %}
      		XYZ
      	{% endfill %}
      {% endcomponent %}

      And we get:

      <div>
        hello
        1
          XYZ
      </div>
    2. blocks CANNOT be overriden through the component tag, so something like this:

      {% component "my_comp" %}
      	{% fill "body" %}
      		XYZ
      	{% endfill %}
      {% endcomponent %}
      {% block "inner" %}
      	456
      {% endblock %}

      Will still render the component content just the same:

      <div>
        hello
        1
          XYZ
      </div>
    3. I CAN override the block tags of abc.html if my component template uses extends. In that case, just as you would expect, the block inner inside abc.html will render OVERRIDEN:

      @component.register("my_comp")
      class MyComp(component.Component):
      	template_name = """
      		{% extends "abc.html" %}
      
      		{% block inner %}
      			OVERRIDEN
      		{% endblock %}
      	"""
    4. This is where it gets interesting (but still intuitive). I can insert even new slots inside these "overriding" blocks:

      @component.register("my_comp")
      class MyComp(component.Component):
      	template_name = """
      		{% extends "abc.html" %}
      
      		{% load component_tags %}
      		{% block "inner" %}
      			OVERRIDEN
      			{% slot "new_slot" %}
      				hello
      			{% endslot %}
      		{% endblock %}
      	"""

      And I can then pass fill for this new_slot when rendering the component:

      {% component "my_comp" %}
      	{% fill "new_slot" %}
      		XYZ
      	{% endfill %}
      {% endcomponent %}

      NOTE: Currently I can supply fills for both new_slot and body slots, and I will not get an error for an invalid/unknown slot name. But since body slot is not rendered, it just won't do anything. So this renders the same as above:

      {% component "my_comp" %}
      	{% fill "new_slot" %}
      		XYZ
      	{% endfill %}
      	{% fill "body" %}
      		www
      	{% endfill %}
      {% endcomponent %}

@JuroOravec
Copy link
Collaborator Author

I've included the explainer in docs/slots_and_blocks.md

@JuroOravec JuroOravec changed the title refactor: fix components and loader tags compat fix: loader tags compatibility May 2, 2024
@JuroOravec JuroOravec merged commit e566d8e into EmilStenstrom:master May 2, 2024
6 checks passed
@JuroOravec JuroOravec deleted the 134-fix-components-and-loader-tags-compat branch May 2, 2024 20:24
@EmilStenstrom
Copy link
Owner

I just wanted to say that this is great work again. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make {% component %} play well with {% block %}
2 participants