Use new `{% slot %}` and `{% fill %}` tags to enable nested components

[EmilStenstrom]

Nested components does not work today, and I think this might be a common case for people using components in bigger projects.

When nesting components, you quickly run into the problem with not knowing if a slot tag refers to you defining a new slot, or if you're filling in an existing slot. This could be solved by introducing attributes that toggle if a slot should behave one way or the other, but I think the problem is fundamental enough that we should update the syntax.

Let's use {% slot %} for defining new slots, and {% fill %} for well... filling a slot with content. This does break backwards compatibility, and requires that people change their code when upgrading, but I think it's worth it to get this right.

Here are the two examples using this syntax. See if you can understand what it's doing before reading the explanation below.

<div id="dashboard">
{% component_block "calendar" date="2020-06-06" %}
    {% fill header %}
        {% slot calendar_header %}{% endslot %}
    {% endfill %}
    {% fill body %}Here are your to-do items for today:{% endfill %}
{% endcomponent_block %}
<ol>
{% for item in items %}
    <li>{{ item }} </li>
{% endfor %}
</ol>
</div>

Explanation: There's an existing component named calendar that contains two slots, header and body. This template code redefines header as new slot called calendar_header, and fills the body slot with some content. Below the component it defines some other content that needs an items parameter. So this is likely the definition of a new component with just has one new slot and takes one parameter items.

Another example, now defining new default content for a slot; it works exactly as non-nested slots.

<div id="dashboard">
{% component_block "calendar" date="2020-06-06" %}
    {% fill header %}
        {% slot calendar_header %}Welcome to your dashboard!{% endslot %}
    {% endfill %}
    {% fill body %}Here are your to-do items for today:{% endfill %}
{% endcomponent_block %}
...

This syntax just makes sense to me. I can fully understand what's happening without any other explanation. It also doesn't require learning any new concept when going from the normal slot and fill concepts to the nested form. Things just work.

[lemontheme]

Just out of curiosity, has there been any progress on this feature? I'm considering adopting django-components for a new project, but my requirements involve component nesting.

[EmilStenstrom]

@lemontheme I’m afraid not. If you can help out I would be much appreciated!

[lemontheme]

Quickly scanned the code base yesterday to guage what sort of effort helping out might entail. Not a Django expert by any stretch of the imagination, but I think I more or less understand what's happening.

Follow-up question so I know what I'd be getting into: Were there any particular challenges you faced when trying to implement this, or have you simply not had the time/need to start on it?

[EmilStenstrom]

No specific challenges, just lack of time. If you stumble on any problems, use this thread for questions and I'll try to help the best I can.

[lemontheme]

So, after taking some time to familiarise myself with the code base and with the internals of Django's parser, I'm happy to report I've made some progress on this over on this branch.

I've added a new templatetag test that demonstrates the core functionality illustrated in your original post. As expected, most of the existing templatetag tests fail, since these haven't been updated to use {% fill %}-blocks. I'm holding off on updating the documentation until we've been able to discuss things in more detail.

Right now, I'm trying to figure out a strategy to introduce this new style of slot filling gradually, in such a way that avoids causing backwards incompatibility. The idea is to be able to use 'fill'-tags alongside 'slot'-tags used with the same meaning (i.e. 'find and fill this slot on the parent component block with the enclosed content'). Obviously, 'slot'-tags have a dual meaning (i.e. 'fill slot' vs. 'declare slot'). What I'm thinking is we should try to detect when a 'slot'-tag is likely being used in the 'fill slot' sense and issue a deprecation warning. I say 'likely' because in some cases the ambiguity is unresolvable (otherwise we wouldn't even be here). All in all, the approach I'm describing means a smoother upgrade route for users at the cost of more lines of code.

Alternatively, it may also just be the case that a hard 'break' with the previous, ambiguous 'slot'-tags makes more sense. It makes the documentation simpler, the code easier to maintain and – on a dogmatic level – keeps us in line with the wisdom of 'there should be one way to do it'.

[EmilStenstrom]

Hi! Sorry for the late reply, forgot! :)

This looks great to me. Keep up the good work! :)

I think we can go for a hard break, and a major version bump. Or, since this library is a 0.X version, we would break backwards compatibility in a minor version. There would be some work for library users to upgrade, but it isn't THAT hard imo. In other words, go ahead and break existing code!

[lemontheme]

No worries. Glad to hear back from you!

All in all I welcome your assessment. It's certainly doable to support both options in parallel (to a point anyway). I'm just not sure if it's worth the added complexity, not least if it risks getting in the way of tackling other open issues.

Here's what I think my next steps are. Let me know if I've missed anything.

• Update all tests to use {% fill %}
• Implement a check for (possible) 'fill'-sense usages of {% slot %} and issue a warning in response.
• Update the documentation, including examples.

Once that's done I'll rebase and open a PR.

As for whether this is a minor or major upgrade, I'm still on the fence. On the one hand, there are open issues that I think should be dealt with before a major release. Also, a major release brings with it certain promises that translate to more maintenance effort. On the other hand, I think we've gotten accustomed to libraries providing deprecation warnings, even at the alpha stage. Regardless of what semver says, sneaking a breaking change into a minor release of alpha software might still end up pissing a few people off... 60% of me says bite the bullet and do it as a minor release, but it's up to you.

I'll let you know when I'm done with the to-dos above!

[EmilStenstrom]

I think it will be easier in the long run to support just one path (fill and slot), than two separate ones (fill/slot and slot). It might be annoying to existing users, but there are not THAT many users, so I think it's better to change it now rather than later.

Your list sounds like good steps. If possible, it would be nice if you added tests for nested components too. In theory, it should be possible to nest slots inside fills with your changes. But there might be more work needed to make that work. There's an example in the top of this PR of how I had imagined it could work.

I think we can do a minor release, but at a new header to top of the README, coupled with your warning message, to point people in the right direction. Maybe also a wiki page with instructions on how to upgrade. I can do all that, if you take care of the code :)

[lemontheme]

Alright, sounds good!

Just got done updating the tests. Everything's green again. Up next is adding tests for nested components. Makes sense to include those, since why else go to all this trouble.

A question that came up while updating the tests is what to do with {{ slot.super }} (if anything)?

For one thing, I noticed it makes refactoring less easy, since you can't just ctrl-D and replace all occurences of 'slot'. I also found it unintuitive to have 'slot'-related variables in a 'fill' context but not with the new, more restrictive 'slot' meaning. By contrast,{ fill.super } posed less friction and felt more natural, with the important exception that '.super' no longer seems appropriate.

The '.super' suffix/attribute made sense when {% slot %} behaved somewhat analogously to Django's 'blocks', where blocks override parent template blocks through inheritance and and access parent block information through 'block.super', mimicing Python's super() function. But with the new distinction between 'fill' and 'slot' tags, there is no inheritance relation, and so the analogy with Django blocks falls away.

Two alternatives that I believe convey the intended meaning more clearly are {{ fill.default }} and {{ slot_default }}. Better yet perhaps – instead of using a variable for this, change it to a tag, e.g. {% slot_default %}.

What are your thoughts on this?

[EmilStenstrom]

Super happy over the progress here! 🚀 👏

Totally agree on the slot.super uglyness. The only reason to have fill.super now is that it mimics how blocks work, so it might be more intuitive to seasoned Django developers. Not sure how important this is, components are already different enough. One convenience thing could be to use .default, but alias .super to .default... Unsure.

About the other options, I'm reading through the open issues to see if any of the other changes might affect the choice of API for this:

• https://github.com/EmilStenstrom/django-components/issues/98 - A way to check of a slot is empty, and build conditional template logic based on that. Suggested syntax: {% if slots.subtitle %} - Should maybe be "filled.subtitle" now?
• https://github.com/EmilStenstrom/django-components/issues/189 - A way to say that a slot is optional to fill, so you can avoid errors when it's not (works nicely with the empty check above). Suggested syntax is {% slot optional %}.
• I have some ideas of generating an unique ID per component, so that templates can access this, and we can do cool stuff on the client side in the future. Maybe {{ fill.id }} could be the API here, similar to your {{ fill.default }} suggestion?

Overall, I think I like the idea of having a variable "fill", that we can add stuff to in the future, without making up a new API.

[lemontheme]

A lot of interesting points here.

Starting with .super, I suspect the way we're using it now would have the effect of being familiar but confusing because, despite what its name implies, .super no longer relates to inheritance. I think we can agree that part of good API design is not mapping too many concepts onto the same term (although things can get subjective quick... ).

I'm struggling to convince myself about 'fill' as a variable. The main reason is that I interpret 'fill' more as a directive: When I see {% fill header %} I read it as fill(slot=header, content=<body of the block tag>). Under this interpretation, associating attributes with 'fill' makes as much sense as associating attributes with a function. An alternative interpretation is to read {% fill header %} as FillClass(slot=header, content=<body of the block tag>). This is more amenable to using 'fill' as a variable. In this sense, {{ fill }} refers to the 'self' of an instance of FillClass.

Full disclosure though: I have a bias against injecting extra variables. They're harder for an IDE such as Pycharm to inspect compared to actual tags, and they're more likely to conflict with user context data.

Here's my counter-proposal. (This is all for the sake of discussion btw. Your project, your vision.)

• {% fill %}..{% endfill %} is simply a block tag.
• To include default slot contents, we could have{% slot_default %} or {% filled_slot.default %} or {% current_slot.default %}. Variable alternative: {{ slots.<slot_name>.default }}, e.g. {{ slots.header.default }}
• Wrt #98, we could introduce a block tag such as {% if_filled <slot_name> %}..{% endif_filled %}. Pro: scope is clear. Cons: yet another tag to memorize; it doesn't reuse the builtin 'if' tag; it's less extendable. Variable alternative: {{ slots.<slot_name>.is_filled }}.
• For generated IDs, I'd again do something with tags, e.g. {% component_id %} or {% id_context %}..{{ id }}..{% endid_context %}... Although, granted, this composes less well with other Django tags. IDs may be the strongest case in favor of variables, actually. (A possible source of inspiration: alpine.js.) Variable alternative: {{ slots.<slot_name>.id }}, e.g. {{ slots.header.id }}.

I rather like 'slots' as a variable name, since it's appropriate both in the context of the source component template and in the context of the template consuming the component. 'slots' is scoped to the template, not to any particular 'fill' block. From within a fill block, all slots (and their associated attributes) can be accessed, not just the slot corresponding to the current fill block. (Obvious downside to accessing slots as a attribute is that the slot name can't contain spaces, which is isn't something that is enforced atm.)

As for adding the 'optional' modifier to {% slot %}, that's something I could add quite easily. That said, I think we should consider that not everyone has the expectation that 'unused slots are an error'. In fact, I think it's more reasonable to expect that slots always gracefully fallback to default content. We can please both camps easily though by adding a new config variable... idk... something like 'STRICT_SLOTS', which, when True, will raise an error on unused slots.

[lemontheme]

So, any thoughts about how you'd like to proceed with this?

(Sorry to nag. It's just every time I look away from the code for more than a week, it takes a while to get back into it.)

[EmilStenstrom]

Thanks for reminding me, I confused the discussion on the PR with the issue discussion.

I don't like the idea of expanding the number of tags you need to remember to use the library effectively, so I'm leaning towards including one "slots" variable that has everything you need in it. I see the point about PyCharm not being able to help though.

A (slightly trickier) variant would be that you specify in the tag by which name you will use:

{% component_block "header" with_context "slots" %}
    {% if slots.up.is_filled %}
         <div>{% slot "up" %}{% endif %}</div>
    {% endslot %}
{% endcomponent_block %}

To be clear, I don't expect you to add all the other stuff I linked to, that could be separate PRs by me or anyone else. I just added them as reference, when thinking about what API we want to impose to our users.

... in other words, I like your counter-propsal, with the slots variable. I think it's easy to learn, and quite intuitive. And yes, this means we should enforce stricter rules on component names.

[EmilStenstrom]

Also: I would happily accept you as a co-owner of this project, so that this is no longer "my" project :)

[lemontheme]

Apologies for the slow reply! (The irony isn't lost on me.) Was a more stressful week than I'd anticipated.

So, I actually started on a rough 'slots' implementation last weekend. I made quite a bit of progress initially. In fact I fully expected to have an updated PR by the end of the day. But then I ran into a few issues that made me doubt whether this is the right way to proceed.

I'll go through them using an example below but the TL;DR is that slots means something different in a 'filling' context than in a 'component template' context, and this, combined with the promotion of slots to a template-global variable, creates ambiguity for both implementation and UX. I have some ideas for how we might resolve this, which I'll present at the end.

---

Let's start with #98. If we want to support such functionality with slots, it follows that the scope of slots needs to be promoted to the level of the template. That is, slots is accessible anywhere in the component template. The snippet below shows what that would look like.

{# component_template.html #}
<div class="frontmatter-component">
    <div class="title">
        {% slot "title" %}Title{% endslot %}
    </div>
   {% if slots.subtitle.is_filled %}
    <div class="subtitle">
        {% slot "subtitle" %}Optional subtitle{% endslot %}
    </div>
   {% endif %}
</div>

The effect of this is that a component template can access fields associated with particular slots from both outside and inside slots. The 'inside' part is somewhat problematic, because this gives a slot \<slotname> access to its own content through slots.\<slotname>.default, which gives rise to recursion. We would have to include extra runtime checks to prevent this.

Another effect of a global slots is that there is nothing stopping slots from accessing fields associated with other slots. A user could unwittingly create dependencies among slots. Can you imagine how much fun it would be to write tests for these kinds of shenanigans? =p

There is, however, an even bigger problem. It has to do with the interpretation of slots. In the example above, slots is a interpreted as a namespace that groups slots declared within the current component template. By contrast, when used in the '{{ slot.super }}' sense, slots refers to the slots, not of the current template, but of the included component template.

To illustrate, here's a simple, unambiguous example:

{# templates/app/main_page.html #}
{% load component_tags %}
{% component_block "component" %}
{% fill header %}Custom header {{ slots.header.default }} {% endfill %}
{% endcomponent_block %}

It's clear in the example above that slots.header.default refers to the default content of the 'header' slot of the component 'component'. But what if the component-including template is itself also a component, as shown in the next example?

{# dashboard_component.html #}
{% load component_tags %}
<div class="dashboard-component">
  {% component_block "calendar" date="2020-06-06" %}
    {% fill header %}
      {% slot header %}Welcome to your dashboard! {{ slots.header.default }}  {% endslot %}
    {% endfill %}
    {% fill body %}Here are your to-do items for today:{% endfill %}
  {% endcomponent_block %}
</div>

What does slots.header refer to now? Is it the 'header' slot of the parent component 'calendar', or is it the new 'header' slot declared by the current component template? Sure, one could argue that it's bad practice to have duplicate slot names, but part of the benefit of the new fill–slot distinction is that in theory that isn't something users need to take into account.

My conclusion: template-global slots = bad.

---

I think we need something that's more particular about what data can be accessed from which context. Here's what I'm thinking of now (still some pieces missing though):

1. A 'fill' block should have access to properties of its parent slot, but not via a global slots variable. Instead, its context is updated to include a new variable with the same name as the parent slot. This variable provides all necessary property acess. To reuse an example from above, here's what that would look like.

{# templates/app/main_page.html #}
{% load component_tags %}
{% component_block "component" %}
  {% fill header %}Custom header {{ header.default }} {% endfill %}
{% endcomponent_block %}

Notice that the name of the filled slot, header, is now also a variable. Obviously, this can lead to naming conflicts, so an obvious escape hatch is that slot names can be aliased with an as keyword:

{# templates/app/main_page.html #}
{% load component_tags %}
{% component_block "component" %}
  {% fill header as cool_new_header %}Custom header {{ cool_new_header.default }} {% endfill %}
{% endcomponent_block %}

I've borrowed this idea entirely from Vue's named scoped slots.

2. A 'slot' block does not have access to its own properties, making recursion less of an issue.
3. I have a hunch conditional slots (as in #98) are fairly advanced functionality. I have no data to back this up but I expect them to get less use. I would still recommend using custom tags for this after all.

Idea 1:

{# component_template.html #}
{% load component_tags %}
<div class="frontmatter-component">

   {% if_filled subtitle %}
    <div class="subtitle">
        {% slot "subtitle" %}Optional subtitle{% endslot %}
    </div>
   {% endif_filled %}
</div>

Idea 2:

{# component_template.html #}
{% load component_tags %}
{% inspect_slots %}
<div class="frontmatter-component">
   {% if subtitle.is_filled %}
    <div class="subtitle">
        {% slot "subtitle" %}Optional subtitle{% endslot %}
    </div>
   {% endif %}
</div>

Idea 3:

{# component_template.html #}
{% load component_tags %}
<div class="frontmatter-component">
   {% if djcomp.is_filled.subtitle %}
    <div class="subtitle">
        {% slot "subtitle" %}Optional subtitle{% endslot %}
    </div>
   {% endif %}
</div>

[lemontheme]

As to your other comment, I'm happy to help out – if only as a way to repay you for all the time your CoNLL-U parser saved me over the years. =p At the same time, though, I can't stress enough how new to Django I still am, so there's still a lot to learn/temptation from React/Vue.js to resist haha

If you like we can set up a call to discuss it, as well as maybe brainstorm our way out of this issue thread.

[EmilStenstrom]

Thanks for tirelessly working on this! That you use and that my CoNLL-U parser saved some time for you makes me even happier!

I really like the idea of:

{% fill header as cool_new_header %}

I think that even accessing .default should considered advanced usage, so I think it makes sense to don't add any variable automatically, and instead only allow access when you have added an alias. Should make this a slightly safer change too.

I'm convinced using a new tag for the "check if slot is filled" case makes sense. Let's go with Idea 1:

{% if_filled subtitle %}

Do you agree that this is a good path forward?

(About your Django experience: Make changes deep inside the template engine like you've did here is not something a beginner can do. You are no beginner, I can tell you that :))

[lemontheme]

Yeah, this feels more like the way to go than before. I'll start working on the first part in the linked PR. Once that's done, I'll open a new PR for the conditional slots.

[...] only allow access when you have added an alias.

Nice extension. Hadn't occured to me yet, but I like it! Should mean fewer unpleasant surprises when upgrading.

(And thanks for the reassuring words! What can I say – I see a parser, I get excited. Comes with the profession, I guess =p)

[EmilStenstrom]

And we're live!

What I did to go live:

• Change the version number on setup.py to 0.26 and commit/push.
• Tag the Bump version-commit with 0.26
• Git push with --tags
• Now PyPI GitHub Action triggers, and package goes live

(Of course, I first tagged the commit before the Bump version commit, which made the 0.25 get pushed to pypi again. That failed because it already exists. Then I found the we've missed updating the release notes in the beginning of the readme and updated that. But that does not show up on the PyPI page, so a made a new point release with only the readme changed. So 0.26.1 is the current latest release!)

[lemontheme]

Awesome! Feels good to get this out the door. =D The README is looking 👌

Thanks for writing down the steps. Copied them to a note for future reference