twigphp / Twig

Twig, the flexible, fast, and secure template language for PHP
https://twig.symfony.com/
BSD 3-Clause "New" or "Revised" License
8.13k stars 1.24k forks source link

[Feature Proposal] Default block in embed #3253

Open benface opened 4 years ago

benface commented 4 years ago

Sometimes an embed only needs one block, and it would be nice if we could shorten this:

{% embed 'widget' %}
  {% block content %}
    Content here
  {% endblock %}
{% endembed %}

to this:

{% embed 'widget' %}
  Content here
{% endembed %}

I would imagine widget.twig to include something like this:

<div class="widget">
  {% block default %}{% endblock %}
</div>
oles commented 3 years ago

I think this is a great idea! I'd love to cut down on the boilerplate for simple embeds. Quite similar to how Vue does it: https://vuejs.org/v2/guide/components-slots.html#Slot-Content

CraftCMS recently added their own custom tag for something similar: https://craftcms.com/docs/3.x/dev/tags.html#tag

suited for cases where the tag contents need to be dynamic.

{% tag 'p' with {
    class: 'welcome',
} %}
    Hello, {{ currentUser.friendlyName }}
{% endtag %}

{# Output: <p class="welcome">Hello, Tim</p> #}
stof commented 3 years ago

This would require a BC-breaking change to the way the {% embed %} tag works. Child templates are allowed to have some kind of content outside top-level blocks.

Also, I fear that parsing that would be quite hard: you will need to parse the body before knowing whether it should be parsed as wrapped in a block or as top-level content.

acalvino4 commented 1 year ago

I second that the way Vue handles this seems to be ideal. Given that they do it, it should be possible, even if difficult.

I would imagine that the general strategy would be to just start parsing the embed contents into a "default" buffer (which becomes the default node), which is paused and then resumed whe encountering block/endblock tags. Basically all content not inside an explicit block becomes part of the default slot.

So what the vue docs show as

<BaseLayout>
  <template #header>
    <h1>Here might be a page title</h1>
  </template>

  <!-- implicit default slot -->
  <p>A paragraph for the main content.</p>
  <p>And another one.</p>

  <template #footer>
    <p>Here's some contact info</p>
  </template>
</BaseLayout>

would become this in twig

{% embed 'BaseLayout.twig' >
  {% block 'header' %}
    <h1>Here might be a page title</h1>
  {% endblock %}

  {# implicit default block #}
  <p>A paragraph for the main content.</p>
  <p>And another one.</p>

  {% block 'footer' %}
    <p>Here is some contact info</p>
  {% endblock %}
{% endembed %}

As in Vue, you could also label that default slot explicitly

  {% block 'default' %}
    <p>A paragraph for the main content.</p>
    <p>And another one.</p>
  {% endblock %}

As far as the edge case where someone has an implicit and explicit default block, well, then I think we'd throw an error/warning - Vue gives us this warning in that case: Extraneous children found when component already has explicitly named default slot. These children will be ignored.

@stof - what do you mean by the following?

Child templates are allowed to have some kind of content outside top-level blocks.

If i do

  {% embed 'component' %}
    Some text outside a block
    {% block content %}My child content{% endblock %}
  {% endembed %}

I get the error A template that extends another one cannot include content outside Twig blocks. So I don't think I understand what you're getting at.

In any case, if I understand the current behavior correctly, content inside an embed but outside a block throws an error. The proposed change would be to simply parse that content into a "default" node that can be accessed inside the child template as `{% block 'default %}. As far as I can tell, this wouldn't cause a breaking change bc all we are doing is defining a use for something previously undefined, namely, "content inside an embed but outside a block".

Edit:

@stof - I think I see what you're getting at now. I can do

  {% embed 'component' %}
    {% set a = 'test' %}
    {% block content %}My child content{% endblock %}
  {% endembed %}

and get no errors. Under our proposed feature implementation, there would then be ambiguity about whether that set call is part of the default block or not. I think there probably are alternative ways to code most things like this, for instance, in my example the set call could be moved outside the embed tag - but requiring this would be a breaking change. Does that summarize what you were getting at?

If that's the case, we have a couple options to get this functionality:

  1. just wait till the next major version, and implement this breaking change in that release. As far as I am aware there is no Twig 4 planned, so this could take a long time.
  2. create a new tag. probably undesirable because include/embed/macro/extends is already a lot, especially for beginners. But if it was agreed that this feature is desired, we could create a new tag and inform users that 'embed' will be deprecated/replaced in the next major version, while of course continuing to support it till then.
  3. Come up with semantics that distinguish default slot code from other non-outputting code. Whatever this would mean, it seems unlikely that it would have any improvement over just declaring the default block explicitly.

Unless there is an upcoming twig 4 release of which I am unaware, option 2 seems like the best option, but I'm not sure if there is any openness to that.

acalvino4 commented 1 year ago

Some additional notes on why I think this behavior is important -

If you are not just using tailwind because it's the hype, but actually following the philosophy, which requires the ability to extract things to components, this use case becomes more relevant. And I think this is true for many others who really try to keep things modular. Of course, we can already do this with embeds using an explicit default/content block, but I've observed too many times that the less straightforward this is, the more likely jr. devs are to start writing custom classes to group css on a tailwind project, which is of course an anti-pattern.

But I think the more important and broader reason is that the feature suggestion here really is the base case. Regarding @benface's initial claim that "Sometimes an embed only needs one block", I might dare say this is most of the time. Anyway, it would seem to me there is a reason that Vue's docs cover what I'm calling the "base case" first, and only later describe "named slots"; or that react has the children construct and doesn't even support multiple children slots without additional code/packages. Of course, the experience and opinions of some may differ on this point, and I don't intend to start a a debate about what's more common - I just want to propose that this is a super common use case for many people and handling it well would help clean up a lot of code out there, if only in a modest way.

I think this small change (small in api, if not in implementation) will go a long way to making embeds feel much more light-weight to use, and to keep templates more readable and modular.

stof commented 1 year ago

The fact that it makes existing code ambiguous is precisely the blocker: it cannot be implemented in a backward compatible way.

felds commented 4 months ago

I think this code

{% embed 'component' %}
    {% set a = 'test' %}
    {% block content %}My child content{% endblock %}
{% endembed %}

could be replaced by this one

{% embed 'component' with {a: 'test'} %}
  My child content
{% endembed %}

and that could make it clear that the variable is being declared on the embed, and not on the default block.


The problem is that reusing variables becomes is a little trickier:

{# This doesn't work because `a` and `b` don't exist yet in the scope when assigning `c`. #}
{% embed 'component' with {a: 1, b: 2, c: a + b} %}

{# This would work, but now the variables are in the current scope, not in the embed scope,
    and could replace other existing variables.
    Also, they are passed in implicitly, which I don't think it's a good practice.  #}
{% set a = 1 %}
{% set b = 2 %}
{% set c = a + b %}
{% embed 'component' %}
WebMamba commented 4 months ago

The behavior described here is exactly what was implemented for TwigComponent. For such a need you can simply use AnonymousComponent. When you have the following code:

<twig:Alert>
   <p>Content</p>
</twig:Alert>

And just with the following code Symfony create a block content that you can access in your component template:

{#  templates/components/Alert.html.twig #}

<div class="alert">
   {% block content %}
   {% endblock %}
</div>