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

[TwigComponent] Add 'host' context when rendering embedded components #863

Closed
wants to merge 1 commit into from

Conversation

sneakyvv
Copy link
Contributor

@sneakyvv sneakyvv commented May 11, 2023

Q A
Bug fix? no
New feature? yes
Tickets
License MIT

Problem

It's not possible to access properties and methods of a component that's using embedded components from within the embedded content (or other embedded blocks).

Reproduce

Create a wrapper component, which is including some components that have embedded content

Component.php

final class Component
{
    public function getSomething(): string
    {
        return 'sf-ux rules!';
    }
}

component.html.twig

<twig:foo>
    <twig:bar :deepVar="this.something" />
</twig:foo>

foo.html.twig (given a Foo.php, with empty class, not important)

<div>
    {% block content %}{% endblock %}
</div>

bar.html.twig (given a Bar.php, with empty class, not important)

{{ dump(deepVar) }}

Expected behavior

The template renders like (with some dump markup of course)

<div>
    sf-ux rules!
</div>

Actual behavior

<div>
    null
</div>

Cause

While rendering an embedded component the generated template will contain something like

$props = $this->extensions["Symfony\\UX\\TwigComponent\\Twig\\ComponentExtension"]->embeddedContext("foo", twig_to_array([]), $context);
        $this->loadTemplate("foo.html.twig", "foo.html.twig", 10, "732315364")->display($props);

The $props is the context used when rendering the embedded content, which is being composed by https://github.com/symfony/ux/blob/2.x/src/TwigComponent/src/ComponentRenderer.php#L92.

And there's the problem: the original $context contains this, but when rendering the Foo component this becomes a reference to Foo and the reference to the original Component is lost.
Imo this behavior is perfectly normal after all, knowing how the rendering works now. I.e. this should reference Foo, because the block is being embedded in the template of foo.html.twig, so while rendering Foo, but it makes sense that in the above template you want to be able to access properties from the "host" component, the component which is embedding some other component.

Solution

Add a host entry to the context referencing the component which is embedding the nested components.


PS: I'm not sure whether I ought to classify this as a bug or as a feature. It's perhaps not a bug in the sense that it's just not possible right now. On the other hand, it is imo something that you'd expect to be able to do, so then it might be a bug after all. You tell me 🙂

For now I classified it as a feature. If it shouldn't be I would have to remove the CHANGELOG entry.

@kbond
Copy link
Member

kbond commented May 11, 2023

parent instead of host maybe? I didn't immediately understand what host was referring to.

@kbond
Copy link
Member

kbond commented May 11, 2023

I would expect:

<twig:foo>
    <twig:bar :deepVar="this.something" />
</twig:foo>

To work though, since you are in Component's template.

I think this is related to #844, @weaverryan, can you confirm?

@weaverryan
Copy link
Member

I actually agree with @kbond:

{# component.html.twig #}
<twig:foo>
    <twig:bar :deepVar="this.something" />
</twig:foo>

I would imagine that this is referring to the Component on the outside. So the proper behavior for me is that this works. To say it differently: when we're inside Component.html.twig, regardless of whether we're rendering other components, we should not do anything special with the variables. If there is some component framework that does it differently, let me know. But conceptually, the above Twig code should work the same as the below PHP code: $this === Component:

class Component

public function render()
{
    $this->renderComponent('foo', [
        'content' => function() {
            $this->renderComponent('bar', [
                'deepVar' => $this->something(),
            ]
        }
    ]);
}

And yep! @kbond I'm 99% sure this is related to #844.

@sneakyvv
Copy link
Contributor Author

parent instead of host maybe? I didn't immediately understand what host was referring to.

@kbond I purposely stayed away from parent, since the (content) block(s) are embedded and not normal extended blocks. I.o.w. there is no parent relationship, if looked at it from an embed perspective. That's actually why it's understandable that this is not working, or not referring to the host/parent Component.
However, I has to ask ChatGPT to come up with a word for "the thing where something is being embedded into". 😄
So, suggestions for a better term are welcome of course!

I would imagine that this is referring to the Component on the outside.

@weaverryan @kbond I understand the expectation that this ought to refer to the Component of the template you're in, as it was what I was expecting initially as well, and I can still see that that perhaps ought to be the desired behavior, especially considering Ryan's example of rendering in PHP.
The problem is of course that internally embedded blocks are being used, and a new template is being rendered "independently" from the parent/host context.

That being said, walking away from using embedded blocks, and using a separate lexer mechanism as suggested in #844 could be a solution, but then it's not possible to refer to the Embedded Component itself.
See the example in my test cases in the commits. I'll repeat it here:

<twig:embedded_content_foo>
    host from embedded content in Foo element: {{ host.something }}
    this from embedded content in Foo element: {{ this.something }}

    <twig:embedded_content_bar :propGivenThis="this.something" :propGivenHost="host.something" id='1' >
        <twig:embedded_content_bar :propGivenThis="this.something" :propGivenHost="host.something" id='2' />
    </twig:embedded_content_bar>
</twig:embedded_content_foo>

So, you could look at the problem as: "it's not a bug, it's a feature!" 🙂

@sneakyvv
Copy link
Contributor Author

sneakyvv commented May 12, 2023

Note that the TwigPreLexer is turning

<twig:foo>
    <twig:bar :deepVar="this.something" />
</twig:foo>

into

{% component 'foo' %}
    {% block content %}
        {{ component('bar', { deepVar: this.something }) }}
    {% endblock %}
{% endcomponent %}

while

{{ component('foo') }}
{% block content %}
    {{ component('bar', {deepVar: this.something}) }}
{% endblock %}

would work, i.e. the reference to this would be correct if the block was outside {% component %}. (edit: it does not work... it's just that the content block is rendered in-place, not as a replacement of the content block of foo. Nevertheless, the other comments below remain the same)
Again, this is because {% component 'foo' %} is interpreted as an embedded component and transformed into $this->extensions['Symfony\UX\TwigComponent\Twig\ComponentExtension']->embeddedContext('foo', ...), which has its own universe/context where this has its own meaning, afaics.

I'm just saying that perhaps we need to understand what is happening within the "all components exist in their own isolated universe" paradigm. If you look at it like that, it completely makes sense that this isn't working.
And concerning @weaverryan's php example, I think we shouldn't confuse this with PHP's $this. I would even say that that example, for it to be similar to the twig syntax, should have been something like

$this->renderComponent('foo', [
        'content' => function() {
            $this->renderComponent('bar', [
                'deepVar' => 'this.something',
            ]
        }
    ]);

@kbond
Copy link
Member

kbond commented May 12, 2023

I purposely stayed away from parent, since the (content) block(s) are embedded and not normal extended blocks. I.o.w. there is no parent relationship

Yeah, I thought about this. In the context of html, parent is the correct term I think:

<div>
  <p>foo</p>
</div>

div is the parent of p...

How do other front end frameworks handle this issue (both logically and naming)?

@sneakyvv
Copy link
Contributor Author

Parent is perfectly fine by me, but I guess we first have to make sure that the compilation is still using the embed flow (see #844 (comment)).

@sneakyvv
Copy link
Contributor Author

sneakyvv commented May 12, 2023

Seeing @weaverryan's latest comment in the related issue (#844 (comment)) and his & @kbond's desire to have this refer to the originating component, I could adapt this PR to simply not override the this-context (see https://github.com/symfony/ux/pull/863/files#diff-d00c28cb67fd375018bb93d026be3ca940b04fe867eeeba76f235eb74279e897R92).
Or are you guys preferring to be able to refer both parent ànd this? See example above

@weaverryan
Copy link
Member

Tbh, I'm not super motivated to make changes to the existing ComponentRenderer stuff, because we've found that it is lacking in other ways. I'm thinking we should port https://github.com/giorgiopogliani/twig-components more directly over to UX named under a new tag like {% twig_component and make THAT work exactly like we want it to. Then we would update the PreLexer to lex into the new {% twig_component and eventually deprecate the current ComponentRenderer / {% component syntax.

@sneakyvv
Copy link
Contributor Author

@weaverryan sounds good!

we should port https://github.com/giorgiopogliani/twig-components more directly over to UX named under a new tag like {% twig_component and make THAT work exactly like we want it to

Is it something I can help with? I have been trying some things the last days, based on https://github.com/giorgiopogliani/twig-components. I'll open a new PR for that if you want to (when it's finished).
In essence we move from an embed node, which uses blocks, to an include node, which subcompiles the slots within the "host/parent" template. That way this always resolves to the expected component, solving the issue addressed in this PR.

I'm not super motivated to make changes to the existing ComponentRenderer stuff, because we've found that it is lacking in other ways

Can you elaborate a bit? It's best to know what should be avoided in the new version.

@sneakyvv
Copy link
Contributor Author

sneakyvv commented May 15, 2023

Sorry guys, but working with it in a real project this Monday, I'm afraid I once again have to change my mind on the embed vs include stuff.

I'll try to recap both options, and give the real example afterwards to support my preference (spoiler: it's embed after all).

Using slots

Mechanism

  • Content is rendered together with the template it's being used in. This means it can use the context of that template.

Syntax (concept)

{# someComponent.twig #}
{% twig_component Foo %}
  some content {# This is used for `content` var in Foo.twig #}
  {{ this.methodCall }} {# This will use SomeComponent::methodCall(), also part of  `content` var in Foo.twig #}
  {% slot namedSlot %}foobar{% slot %}
{% endtwig_component %}
{# Foo.twig #}
<div class="foo">{{ content ?? 'default content' }} {{ namedSlot ?? 'foo' }}</div>

PRO

  • Clear intuitive expectations: this refers to the Component of the template it's used in.

CON

  • No access to context of the component you're overriding/providing the slot of/for.

Using embed (current)

Mechanism

  • Content is rendered inside the template where the block is defined. This means it is using the context of that template. It also gets the context of the parent, but the this reference is overridden, changing it to the new current component. Optionally, the addition of a parent/host context could solve this, which is what this PR is about.

Syntax

{# someTemplate.twig #}
{% component Foo %}
  some content {# This is used for `content` block in Foo.twig #}
  {{ this.methodCall }} {# This will use Foo::methodCall(), also part of  `content` block in Foo.twig #}
  {{ parent.methodCall }} {# (optional) This will use SomeComponent::methodCall(), also part of  `content` block in Foo.twig #}
  {% block namedBlock %}foobar{% endblock %}
{% endcomponent %}
{# Foo.twig #}
<div class="foo">{% block content%}default content{% endblock %} {% block namedBlock %}foo{% endblock %}</div>

PRO

  • Access to context of the component/template you're overriding/providing the slot of/for.

CON

The example to show the benefit of keeping embed

{# Table.twig #}
<table class="{% block table_class %}table{% endblock %}">
    {% block content %}{% endblock %}
</table>
{# Body.twig #}
<tbody>
{% block content %}
{% for row in rows %}
    <tr scope="row">
    {% block row %}
        {% for cell in row %}
        <twig:cell>{{ cell }}</twig:cell>
        {% endfor %}
    {% endblock %}
    </tr>
{% endfor %}
{% endblock %}
</tbody>
{# Cell.twig #}
<td class="{% block td_class %}{% endblock %}">{% block content %}{% endblock %}</td>
{# MyTable.twig #}
<twig:table>
    <twig:head :headers="headers" :withAction="true" :sortableColumns="sortableColumns" />
    <twig:body :rows="host.rows">
        <twig:block name="row">
            {% for cell in row %}
            <twig:cell>{{ cell }}</twig:cell>
            {% endfor %}
            <twig:cell>
                doing something special with the action column
                <twig:block name="td_class">action</twig:block>
            </twig:cell>
        </twig:block>
    </twig:body>
</twig:table>

With an include (slot) syntax, it wouldn't be possible to access the row variable inside MyTable.twig, which is actually from the context of Body.twig. Imho this is a huge pro for the embed flow. I also believe that the con 'confusing context' can be solved with proper documentation.

PS: #844 is imo a case of not properly grasping embed vs include, because it is confusing, but shouldn't require any changes, except for perhaps adding documentation making that difference more clear leading to proper expectations.

@weaverryan
Copy link
Member

Hey @sneakyvv!

I appreciate the objective detailed discovery that you're doing 👍. I admit that I am "partial" towards the "slot" idea, because I find the block hierarchy very confusing. But I'll do my best to stay objective also ;). By the way, I also think there are 2 separate "things" that are being discusses: (A) the embed block hierarchy and (B) the context inside of {% component (i.e. is this the parent component or the child component).

Let me challenge you on one point:

With an include (slot) syntax, it wouldn't be possible to access the row variable inside MyTable.twig, which is actually from the context of Body.twig. Imho this is a huge pro for the embed flow

This refers to:

    <twig:body :rows="host.rows">
        <twig:block name="row">
            {% for cell in row %}
                 ...
            {% endfor %}
        </twig:block>
    </twig:body>

That looks really confusing to me - way too magical. The row variable is coming specifically from the row block of Body.twig. I understand what you're trying to do - you want the code inside of <twig:block name="row"> to function as if it has been copied and pasted directly into THAT block in Body.twig. You're right that Twig DOES allow this and works this way. But it's very non-obvious - the row variable seems to come out of nowhere.

However, I admit that I can't think of a decent solution to this use-case. About #844, if things were kept as they are now, how would the correct code look for doing this:

{# DangerButton.html.twig #}

{% component 'Button' with { type: 'danger' } %}
    {% block content %}{{ block('content') }}{% endblock %}
{% endcomponent %}

That is where someone is calling <twig:DangerButton>Danger</twig:DangerButton> and you want to "forward" the content block (containing Danger) through to the final Button component. Would you document to "Never use the block() function inside of {% component" and instead document this?

{# DangerButton.html.twig #}

{% set outerContent = block('content') %}
{% component 'Button' with { type: 'danger' } %}
    {% block content %}{{ outerContent }}{% endblock %}
{% endcomponent %}

It's not super elegant... /cc @WebMamba gotta include you here, as you're already working on the slot-type. I would still prefer the slot type - but the table/row use-case above deserves a nice solution.

@WebMamba
Copy link
Collaborator

Hey, @sneakyvv thanks to raised all of this!
We can work on some tests in #873, and see if we can find solutions to convince you! 😁
For the Table example, we can simply do a test or build a separate repo using the slot PR.
I follow this discussion from the beginning, and I see the pro and con you talking about. But I just feel even if the slot has cons, it is much more intuitive. I think it's just too early to take a decision, let's play a bit with both approaches! 😁

@WebMamba
Copy link
Collaborator

@weaverryan
Copy link
Member

Thanks @WebMamba - that IS way nicer. Any idea on the table for situation above? I was thinking about this situation this morning, and though I love the simplicity of slots, the current solution is VERY powerful.

so the question is: keep that power and document or try to find a solution to “forwarding” blocks into a component (setting the block to a variable above the component works, but isn’t great… but works) or continue the slot idea and try to find a solution for the table row problem.

@WebMamba
Copy link
Collaborator

Ok so here is my solution for the Table problem.
Let's keep it simple at first, we just want to make a simple table component with, which we give some data and it works:

// index.html.twig

{% extends 'base.html.twig' %}

{% block body %}
    <twig:Table :rows="{foo: ['bar', 'baz']}"/>
{% endblock %}
// /components/Table.html.twig

<table>
    <twig:TableBody :rows="rows"/>
</table>
// /components/TableBody.html.twig
<tbody>
    {% for row in rows %}
        <tr scope="row">
            {% block row %}
                {% for cell in row %}
                    <twig:Cell>{{ cell }}</twig:Cell>
                {% endfor %}
            {% endblock %}
        </tr>
    {% endfor %}
</tbody>
// components/Cell.html.twig
<td class="{{ attributes.merge({class: "td"}) }}">{{ slot }}</td>

Ok now we have our simple component render. But now let's add some difficulties and take into account the @sneakyvv problem. 😁 So we want to add more flexibility to the Table component and give the ability to choose how to render the cell. Here is my solution for it:

// index.html.twig

{% extends 'base.html.twig' %}

{% block body %}
    <twig:Table :rows="{foo: ['bar', 'baz']}">
        <twig:slot name="cell">
            <twig:Cell>
                <twig:slot-value name="cell_value"/>
            </twig:Cell>
        </twig:slot>
    </twig:Table>
{% endblock %}

So here we put the cell component into a slot inside the table component. And we use a new syntax <twig:slot-value, lets see later how it work.

// components/Table.html.twig
<table>
    <twig:TableBody :rows="rows">
        <twig:slot name="cell">
            {{ cell }}
        </twig:slot>
    </twig:TableBody>
</table>

Here we just push again the cell component through an other slot

// component/TableBody.html.twig
<tbody>
    {% for row in rows %}
        <tr scope="row">
            {% block row %}
                {% for cell in row %}
                    {{ cell.withContext({'cell_value': cell}) }}
                {% endfor %}
            {% endblock %}
        </tr>
    {% endfor %}
</tbody>

And here we are! We render the slot that contains the cell, but with the function withContext. This function takes an array containing the key: cell_value and the value of the cell. And remember, we used: <twig:slot-value name="cell_value"/> so this gonna be replaced by the cell value, and everything will be rendered as expected.

@weaverryan tell me what you think and I can push a commit to add this withContext method.
This idea came from Reactjs they do something like that when you want to push props to children components, by using the function cloneElement on your child.

@sneakyvv
Copy link
Contributor Author

Hey @weaverryan, @WebMamba,

Sorry for the slow response, we're having a 4-day weekend here in Belgium and good weather so... 😎 😅

Anyhow... Here's my (once again, long (sry)) feedback:

That looks really confusing to me - way too magical. [...] you want the code inside of <twig:block name="row"> to function as if it has been copied and pasted directly into THAT block [...]

In my mind that's always how you'd work with blocks, whether in an extend/include or embed way. That is, you'd look at the template you're using/overriding and replace some content within it, so indeed as if it's copied right in there. Most of the time those blocks don't really use the context where they're being replaced into, so the "magic" usage of those context variables isn't that common, but as you also noted it is a VERY powerful feature. Perhaps, again referring to the component's own "universe" helps to understand the magic (or not being confused by it).

Now, about the content-in-content block issue (for now leaving @WebMamba's comments out of the equation):

Would you document to "Never use the block() function inside of {% component" and instead document this?

I love the simplicity of slots, the current solution is VERY powerful.

so the question is: keep that power and document or try to find a solution to “forwarding” blocks into a component (setting the block to a variable above the component works, but isn’t great… but works) or continue the slot idea and try to find a solution for the table row problem.

I was thinking that the fact that a content block is being use inside a component could perhaps be detected and that the variable assignment in that case could be automized? Not sure if would be that easy though...

I wouldn't necessarily document "Never use the block() function inside of {% component", or at least not as such.
The thing #844 is trying to do is wrap some (content) blocks with some HTML which is already done by some other "atom" component. Generally speaking it boils down to

{# Template A #}

<twig:B>
    My inner content for B {# content defined at top level #}
</twig:B>

{# Template B (component) #}

<twig:C>{%- block content -%}{%- endblock -%}</twig:C> {# passing content to a C component #}

{# Template C (component) #}

<div class="dressing up the content a bit in an atom fashion">{%- block content -%}{%- endblock -%}</div>

The problem is actually that the content of a twig block is automatically used as a content block. It's from there that the logical consequence ensues that you shouldn't use a content block inside a twig component instance. So therefore, I would argue that you don't need to document the "logical thing". However, since this kind of (atom) composition will be a very common use case, I would document it perhaps within that frame of reference. That in those cases, i.e. cases where you have a twig component instance (C) inside a twig component's (B) template, and you want to define the content of C from template A, that in those cases you need to pass the content along via a new variable because (indeed) you cannot ever use a content block inside a component, because that is being translated into content-in-content, which is illegal. Either that or just use another name for the inner-content block, but then again that's not elegant to use in template A, because you can't write it as the body of a component (using the default content block).

Ok, then finally, I want to address @WebMamba's approach:
First of all, let me start with saying that I love all the effort that everyone is putting into this. A token of appreciation once in a while never hurts 😉

Second of all,

take into account the @sneakyvv problem

I just love having a problem being named after me 😂

Ok, let's dive into the proposed solution. Right off the bat... Let me confess, I'm having troubles understanding it 🙈
I get that you're overriding the cell slot in index.html.twig and you're just passing it along in Table.html.twig to TableBody.html.twig, but how cell_value is getting it's value and what withContext does isn't clear to me (without having looked at the code).

A few things also came to mind seeing this example:

  1. The first thing isn't really related to the specific example, but the withContext function made me think: How would you be able to see in a template whether some variable is a "slot variable" or a regular variable, either from the template or Component scope?
    The (for me) normal workflow of looking at a template and immediately seeing what parts can be overridden is not possible, right?
  2. The table solution seems to me like it's reversing the way components are composed. I assumed that you would always have some kind of atomic design, where smaller components are being used to build bigger and bigger blocks, reusing the smallest components many times. If I understand the solution to the @sneakyvv problem (😉) correct, then the index.html.twig & Table.html.twig are altered to make the solution possible, while they should be kept agnostic. At most they provide some blocks/parts that CAN be overridden, but not for example that the Table component's template is defining a cell slot, just so that some higher component can override the lower component's slot. That would quickly become unmaintainable as it would have to provide any slot that may be overridden at some point.

PS: I don't have any stakes in either solution, so don't take my comments as me trying to pull down the slot solution. I'm probably just missing the key part about it :-)
And to reiterate, I would love the simplicity without losing the power of blocks, keeping all of its benefits and not introducing a new layer of complexity to allow slots to do what blocks already can do.

@WebMamba
Copy link
Collaborator

Here is a comment to resume my thought about all of this.
I just feel like Slot is way simpler and more intuitive to use, and this is even more true for people not familiar with Twig. But of course yes, it's less powerful by default than the block approach.
Your Table problem @sneakyvv is totally legit, and if you choose the slot approach we need to address a fix for it. But this problem those not represent how people gonna use the TwigComponent in their daily tasks. If we just look at Reactjs or Svelte, everything works more like Slot.
So it sounds better for me to remove this complexity not needed most of the time and to give some solutions for it when people need it. With slot we can let people choose when they want this complexity or not. 😁

@weaverryan
Copy link
Member

Hi!

Apologies for being a bit slow/absent. @WebMamba while I agree with what you said (I really like the simplicity of slots), I've become convinced of the power of the block case... and I think we should try to make it work first. But, it would require 3 things:

  1. A way to get the "parent" context (this PR)
  2. Documentation update to teach people that, when you are writing content inside of a <twig:Component>, you should pretend that you are now writing in a totally separate template that extends the component's template.
  3. We need a decent way to "forward" blocks. The current work-around is:
{# anyTemplate.html.twig #}

<twig:SuccessAlert>You did it!</twig>
{# SuccessAlert.html.twig #}

{# the workaround: you must set the block to a variable *outside* of the block first #}
{% set content = block('content') %}
<twig:Alert type="success">
    {{ content }}
</twig>

This last one is a serious problem and I think we need to come up with a decent solution for it in order to move forward with the the block-based system. Any ideas on what the proper syntax should look like?

Or... do both?

Btw, another idea would be to combine the idea of slots and blocks where "slots" are the default mode, but you could also use blocks for advanced use-cases. Something like:

{# anyTemplate.html.twig #}

<twig:SuccessAlert>You did it!</twig>
{# SuccessAlert.html.twig #}

<twig:Alert type="success">
    {{ slot }}

    {% block footer %}
        I can print the {{ type }} var from Alert.html.twig
    {% endblock %}
</twig>
{# Alert.html.twig #}

<div class="alert alert-{{ type }}">
    {{ slot }}

    {% block footer %}{% endblock %}
</div>

I kinda like this :). I think it would just require that the new twig_component from #873 be built "inside" of a structure that is very similar to the current {% component - i.e. one that leverages the "embed" type of magic.

@WebMamba
Copy link
Collaborator

WebMamba commented Jun 1, 2023

No worries @weaverryan! That was for a good cause, I m eager to try the AssetComponent!
I like the idea of combining slot and block, let's try that! 😁
My only concern is about LiveComponent, I think if we only used slot, the embedded live component should work. But by adding the embed magic I think we bringing back the blocking point 🤔

@kbond
Copy link
Member

kbond commented Jun 1, 2023

My only concern is about LiveComponent, I think if we only used slot, the embedded live component should work. But by adding the embed magic I think we bringing back the blocking point

@WebMamba did you take a look at #913?

@sneakyvv
Copy link
Contributor Author

sneakyvv commented Jun 3, 2023

Hi all,

A lot has happened meanwhile. So a quick recap:

  1. Indeed [TwigComponent] [LiveComponent] Add support for embedded live components #913 has been added, which makes live embedded components possible.
  2. [TwigComponent] Support passing blocks to nested embedded components #920 is a solution to [TwigComponent] Impossible to declare higher level component content block #844 — it makes passing blocks down to nested embedded components possible, in an intuitive way. Or, defined the other way around, and more correctly imo: it allows you to override a block that belongs to a nested component from within the body of a parent component, and infinitely deep.

Making that last change made me realize that this PR is insufficient, if #920 is to be merged, because it would bring the need to access the nested component hierarchy more than one level up (which is now host/parent). You can imagine you would need to have access to the grandparent variable for example, or even higher up, which actually (probably but not necessarily) would refer to the template containing the block override.
Anyhow, I have already experimented with a Hierarchy object that acts as a proxy object for a Component, which makes something like this.parent.parent.parent.someProp possible.
I'll await the feedback about #920 and this proposal to push that into this PR, or perhaps create a separate PR altogether.

@sneakyvv
Copy link
Contributor Author

Making that last change made me realize that this PR is insufficient, if #920 is to be merged, because it would bring the need to access the nested component hierarchy more than one level up (which is now host/parent).
[...]
I have already experimented with a Hierarchy object that acts as a proxy object for a Component, which makes something like this.parent.parent.parent.someProp possible. I'll await the feedback about #920 and this proposal to push that into this PR, or perhaps create a separate PR altogether.

I created a separate PR for this => #936 (which continues on the changes of #920)

weaverryan added a commit that referenced this pull request Sep 27, 2023
…endering embedded components (sneakyvv)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[TwigComponent] Add variable to refer outer scope when rendering embedded components

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| Tickets       |
| License       | MIT

This PR's is an alternative approach to the problem stated in #863.

Due to the changes proposed in #920, which deals with passing blocks down to nested embedded components, I realized that just being able to access the host (or parent) component isn't sufficient if blocks can be passed down multiple levels deep (as already mentioned in #863 (comment)).

This PR adds a proxy object `Hierarchy` which allows this syntax:

`Component.php`
```php
final class Component
{
    public function scream(int $screamHowHard = 1): string
    {
        return 'sf-ux rules' . str_repeat('!', $screamHowHard);
    }
}
```

```twig
{# component.html.twig #}

<twig:Foo :someProp="this.something(1)">
    {{ someProp }}
    {{ outerScope.this.something(3) }}
</twig:Foo>
```

```twig
{# Foo.html.twig #}

<div>{% block content %}{% endblock %}</div>
```

Resulting in

```twig
<div>
    sf-ux rules!
    sf-ux rules!!!
</div>
```

Adding another layer below Foo, would ask for something like

```twig
{# component.html.twig #}

<twig:Foo>
    {{ outerScope.outerScope.this.something(3) }}
</twig:Foo>
```

to still point to Foo's function.

---

### Questions

There's also a `ComputedPropertiesProxy` object. Should we have a cached version of the `Hierarchy` proxy?

---

### NOTE

This PR continues on the changes of #920, because having a hierarchy only makes sense if blocks can be passed down multiple times. So #920 may have to be merged first, but if needed, and the `outerScope.this` syntax is preferred over the syntax of #863, I could push my branch that starts of the main branch instead.

Edited: using outerScope variable now instead Hierarchy proxy

Commits
-------

e618111 [TwigComponent] Add variable to refer outer scope when rendering embedded components
symfony-splitter pushed a commit to symfony/ux-twig-component that referenced this pull request Sep 27, 2023
…endering embedded components (sneakyvv)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[TwigComponent] Add variable to refer outer scope when rendering embedded components

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| Tickets       |
| License       | MIT

This PR's is an alternative approach to the problem stated in #863.

Due to the changes proposed in #920, which deals with passing blocks down to nested embedded components, I realized that just being able to access the host (or parent) component isn't sufficient if blocks can be passed down multiple levels deep (as already mentioned in symfony/ux#863 (comment)).

This PR adds a proxy object `Hierarchy` which allows this syntax:

`Component.php`
```php
final class Component
{
    public function scream(int $screamHowHard = 1): string
    {
        return 'sf-ux rules' . str_repeat('!', $screamHowHard);
    }
}
```

```twig
{# component.html.twig #}

<twig:Foo :someProp="this.something(1)">
    {{ someProp }}
    {{ outerScope.this.something(3) }}
</twig:Foo>
```

```twig
{# Foo.html.twig #}

<div>{% block content %}{% endblock %}</div>
```

Resulting in

```twig
<div>
    sf-ux rules!
    sf-ux rules!!!
</div>
```

Adding another layer below Foo, would ask for something like

```twig
{# component.html.twig #}

<twig:Foo>
    {{ outerScope.outerScope.this.something(3) }}
</twig:Foo>
```

to still point to Foo's function.

---

### Questions

There's also a `ComputedPropertiesProxy` object. Should we have a cached version of the `Hierarchy` proxy?

---

### NOTE

This PR continues on the changes of #920, because having a hierarchy only makes sense if blocks can be passed down multiple times. So #920 may have to be merged first, but if needed, and the `outerScope.this` syntax is preferred over the syntax of #863, I could push my branch that starts of the main branch instead.

Edited: using outerScope variable now instead Hierarchy proxy

Commits
-------

e618111be0 [TwigComponent] Add variable to refer outer scope when rendering embedded components
@CMH-Benny
Copy link

CMH-Benny commented Apr 10, 2024

I just stumbled over this PR and it's discussing an exact problem I have and I didn't find a solution yet.

I have quite a complex setup with blocks and components and for some reason, even tho there is the outerBlocks variable, I can't pass normal twig blocks down as content/named block of a component. Example:

List.html.twig

<ul>{% block content %}{% endblock %}</ul>

ListItem.html.twig

<li>
  {% block content %}{% endblock %}
  {% if hasSubList %}
    {{ block(outerBlocks.sublist) }}
  {% endif %}
</li>

Label.html.twig

<span>{% block content %}{% endblock %}</span>

Now I wanna use it in a block based template (think about a form theme for example, or something selfcooked):

{% block list %}
<twig:List>
  {{ block('children') }} // doesn't work
  {{ block(outerBlocks.children) }} // seems to work... but
</twig:List>
{% endblock %}

{% block children %}
  {% for child in children %}
    {{ block('item') }} // also still seems to work... but
  {% endfor %}
{% endblock %}

{% block item %}
<twig:ListItem>
   {{ block(outerBlocks.label) }} // doesn't render anything
   <twig:block name="sublist">
     {{ block(outerBlocks.list) }} // also doesn't render anything
   </twig:block>
</twig:ListItem>
{% endblock %}

{% block label %}
<twig:Label>{{ child.label }}</twig:Label>
{% endblock %}

What am I missing, I would have expected this to work, but it seems it's not working, so there is no way to build components in such a nested way? Any help would be really appreciated 🙏

@smnandre
Copy link
Member

@CMH-Benny it would be really easier for us if you could create an issue (and there link to this PR if you need) instead of reviving old PR/issues, because we can miss them, and their original author receives notifications ;)

I let you reopen one if this issue is still not resolved ? thx

@smnandre smnandre closed this Apr 19, 2024
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.

6 participants