Skip to content

Commit 19feb79

Browse files
committed
bug #1093 [TwigComponent][LiveComponent] Fix DataModelPropsSubscriber for embedded components (sneakyvv)
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [TwigComponent][LiveComponent] Fix DataModelPropsSubscriber for embedded components | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Tickets | | License | MIT ### Reproduce ``` {# components/A.html.twig #} <B> {# content doesn't matter, as long as it's an embedded component #} </B> ``` ``` {# components/B.html.twig #} <C data-model="X"/> ``` (given a component B with a prop X) With this setup the `DataModelPropsSubscriber::preMount()` would result in an error "_Can't get a way to read the property "X" in class "A"_". An even simpler setup ``` {# anyTemplate.html.twig #} <A> {# A is already embedded #} </A> ``` Will result in "_You can only pass "data-model" when rendering a component when you're rendering inside of a parent component._" Whether the data-model attribute is deep inside template B or already in template A doesn't matter. The thing is that during rendering of A, which is now embedded, there's no component on the stack. ### Problem Self-closing component are rendered via `ComponentRenderer::createAndRender()`. Embedded components are rendered via `ComponentRenderer::embeddedContext()` + `Template::display()`. Both push the component being rendered on the `ComponentStack`, and pop it again at the end of the `ComponentRender` functions. During the rendering of C, B is already been popped from the stack, and therefore the `DataModelPropsSubscriber` doesn't have a way to find the parent component context. And it incorrectly uses component A, which is still on the component stack (while it should be linked to B). ### Solution The rendering of an embedded component is only really done after the `Template::display()` execution. Therefore the component can only be removed when BOTH are done. Commits ------- 6933040 [TwigComponent][LiveComponent] Fix DataModelPropsSubscriber for embedded components
2 parents f6809f8 + 6933040 commit 19feb79

12 files changed

+166
-18
lines changed

src/LiveComponent/src/EventListener/DataModelPropsSubscriber.php

+21-2
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313

1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1515
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
16+
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
1617
use Symfony\UX\LiveComponent\Util\ModelBindingParser;
1718
use Symfony\UX\TwigComponent\ComponentStack;
1819
use Symfony\UX\TwigComponent\Event\PreMountEvent;
20+
use Symfony\UX\TwigComponent\MountedComponent;
1921

2022
/**
2123
* Parses the "data-model" key, which triggers extra props to be passed in.
@@ -54,8 +56,9 @@ public function onPreMount(PreMountEvent $event): void
5456
unset($data['dataModel']);
5557
$data['data-model'] = $dataModel;
5658

57-
// the parent is still listed as the "current" component at this point
58-
$parentMountedComponent = $this->componentStack->getCurrentComponent();
59+
// find the first parent of the component about to be rendered that is a Live Component
60+
// only those can have properties controlled via the data-model attribute
61+
$parentMountedComponent = $this->getCurrentLiveComponent($this->componentStack);
5962
if (null === $parentMountedComponent) {
6063
throw new \LogicException('You can only pass "data-model" when rendering a component when you\'re rendering inside of a parent component.');
6164
}
@@ -76,4 +79,20 @@ public static function getSubscribedEvents(): array
7679
PreMountEvent::class => 'onPreMount',
7780
];
7881
}
82+
83+
private function getCurrentLiveComponent(ComponentStack $componentStack): ?MountedComponent
84+
{
85+
foreach ($componentStack as $mountedComponent) {
86+
if ($this->isLiveComponent($mountedComponent->getComponent()::class)) {
87+
return $mountedComponent;
88+
}
89+
}
90+
91+
return null;
92+
}
93+
94+
private function isLiveComponent(string $classname): bool
95+
{
96+
return [] !== (new \ReflectionClass($classname))->getAttributes(AsLiveComponent::class);
97+
}
7998
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\UX\LiveComponent\Tests\Fixtures\Component;
13+
14+
use Symfony\UX\TwigComponent\Attribute\AsTwigComponent;
15+
16+
/**
17+
* @author Bart Vanderstukken <[email protected]>
18+
*/
19+
#[AsTwigComponent('input_component')]
20+
final class InputComponent
21+
{
22+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\UX\LiveComponent\Tests\Fixtures\Component;
13+
14+
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
15+
use Symfony\UX\LiveComponent\DefaultActionTrait;
16+
17+
/**
18+
* @author Bart Vanderstukken <[email protected]>
19+
*/
20+
#[AsLiveComponent('parent_component_data_model')]
21+
final class ParentComponentDataModel
22+
{
23+
use DefaultActionTrait;
24+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\UX\LiveComponent\Tests\Fixtures\Component;
13+
14+
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
15+
use Symfony\UX\LiveComponent\Attribute\LiveProp;
16+
use Symfony\UX\LiveComponent\DefaultActionTrait;
17+
18+
/**
19+
* @author Bart Vanderstukken <[email protected]>
20+
*/
21+
#[AsLiveComponent('parent_component_data_model_2')]
22+
final class ParentComponentDataModel2
23+
{
24+
use DefaultActionTrait;
25+
26+
#[LiveProp(writable: true)] public string $content;
27+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<input{{ attributes }} />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
{% component parent_component_data_model_2 with { content: 'default content on mount' } %}
2+
{% endcomponent %}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
{{ component('textarea_component', { dataModel: 'content' }) }}
2+
{% component input_component with { dataModel: 'content' } %}{% endcomponent %}

src/LiveComponent/tests/Integration/EventListener/DataModelPropsSubscriberTest.php

+33-6
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,16 @@
1313

1414
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
1515
use Symfony\Component\HttpFoundation\Request;
16+
use Symfony\UX\LiveComponent\Tests\LiveComponentTestHelper;
1617
use Symfony\UX\TwigComponent\ComponentRenderer;
1718

1819
final class DataModelPropsSubscriberTest extends KernelTestCase
1920
{
21+
use LiveComponentTestHelper;
22+
2023
public function testDataModelPropsAreSharedToChild(): void
2124
{
22-
// work around so that a session is available so CSRF doesn't fail
23-
$session = self::getContainer()->get('session.factory')->createSession();
24-
$request = Request::create('/');
25-
$request->setSession($session);
26-
$requestStack = self::getContainer()->get('request_stack');
27-
$requestStack->push($request);
25+
$this->fakeSession();
2826

2927
/** @var ComponentRenderer $renderer */
3028
$renderer = self::getContainer()->get('ux.twig_component.component_renderer');
@@ -42,4 +40,33 @@ public function testDataModelPropsAreSharedToChild(): void
4240
$this->assertStringContainsString('<textarea data-model="content:value">Hello data-model!</textarea>', $html);
4341
$this->assertStringContainsString('<textarea data-model="content2:value">Value for second child</textarea>', $html);
4442
}
43+
44+
public function testDataModelPropsAreAvailableInEmbeddedComponents(): void
45+
{
46+
$this->fakeSession();
47+
48+
$templateName = 'components/parent_component_data_model.html.twig';
49+
$obscuredName = '684c45bf85d3461dbe587407892e59d8';
50+
$this->addTemplateMap($obscuredName, $templateName);
51+
52+
/** @var ComponentRenderer $renderer */
53+
$renderer = self::getContainer()->get('ux.twig_component.component_renderer');
54+
55+
$html = $renderer->createAndRender('parent_component_data_model', [
56+
'attributes' => ['data-live-id' => 'dummy-live-id'],
57+
]);
58+
59+
$this->assertStringContainsString('<textarea data-model="content">default content on mount</textarea>', $html);
60+
$this->assertStringContainsString('<input data-model="content" value="default content on mount" />', $html);
61+
}
62+
63+
private function fakeSession(): void
64+
{
65+
// work around so that a session is available so CSRF doesn't fail
66+
$session = self::getContainer()->get('session.factory')->createSession();
67+
$request = Request::create('/');
68+
$request->setSession($session);
69+
$requestStack = self::getContainer()->get('request_stack');
70+
$requestStack->push($request);
71+
}
4572
}

src/TwigComponent/src/ComponentRenderer.php

+10-9
Original file line numberDiff line numberDiff line change
@@ -93,17 +93,18 @@ public function embeddedContext(string $name, array $props, array $context, stri
9393

9494
$this->componentStack->push($mounted);
9595

96-
try {
97-
$embeddedContext = $this->preRender($mounted, $context)->getVariables();
98-
99-
if (!isset($embeddedContext['outerBlocks'])) {
100-
$embeddedContext['outerBlocks'] = new BlockStack();
101-
}
96+
$embeddedContext = $this->preRender($mounted, $context)->getVariables();
10297

103-
return $embeddedContext;
104-
} finally {
105-
$this->componentStack->pop();
98+
if (!isset($embeddedContext['outerBlocks'])) {
99+
$embeddedContext['outerBlocks'] = new BlockStack();
106100
}
101+
102+
return $embeddedContext;
103+
}
104+
105+
public function finishEmbeddedComponentRender(): void
106+
{
107+
$this->componentStack->pop();
107108
}
108109

109110
private function preRender(MountedComponent $mounted, array $context = []): PreRenderEvent

src/TwigComponent/src/ComponentStack.php

+9-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*
1717
* @internal
1818
*/
19-
class ComponentStack
19+
class ComponentStack implements \IteratorAggregate
2020
{
2121
/**
2222
* @var MountedComponent[]
@@ -60,4 +60,12 @@ public function hasParentComponent(): bool
6060
{
6161
return (bool) $this->getParentComponent();
6262
}
63+
64+
/**
65+
* @return MountedComponent[]|\ArrayIterator
66+
*/
67+
public function getIterator(): \Traversable
68+
{
69+
return new \ArrayIterator(array_reverse($this->components));
70+
}
6371
}

src/TwigComponent/src/Twig/ComponentExtension.php

+9
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,15 @@ public function embeddedContext(string $name, array $props, array $context, stri
8080
}
8181
}
8282

83+
public function finishEmbeddedComponentRender(): void
84+
{
85+
try {
86+
$this->container->get(ComponentRenderer::class)->finishEmbeddedComponentRender();
87+
} catch (\Throwable $e) {
88+
$this->throwRuntimeError($name, $e);
89+
}
90+
}
91+
8392
private function throwRuntimeError(string $name, \Throwable $e): void
8493
{
8594
if (!($e instanceof \Exception)) {

src/TwigComponent/src/Twig/ComponentNode.php

+6
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,12 @@ public function compile(Compiler $compiler): void
8585
$compiler->raw('->display($embeddedContext, $embeddedBlocks);');
8686
$compiler->raw("\n");
8787

88+
$compiler->write('$this->extensions[')
89+
->string(ComponentExtension::class)
90+
->raw(']->finishEmbeddedComponentRender()')
91+
->raw(";\n")
92+
;
93+
8894
$compiler
8995
->outdent()
9096
->write('}')

0 commit comments

Comments
 (0)