[TwigComponent][LiveComponent] Fix DataModelPropsSubscriber for embedded components #1093
Conversation
sneakyvv
force-pushed
the
fix-embedded-component-stack
branch
from
d8ac2e1
to
24338fb
Compare
|
Triggered a test re-run via force-push. |
| private function getCurrentLiveComponent(ComponentStack $componentStack): ?MountedComponent | ||
| { | ||
| $iterator = $componentStack->getIterator(); | ||
| while (($current = $iterator->current()) && !$this->isLiveComponent($current->getComponent()::class)) { |
There was a problem hiding this comment.
Because it now implements IteratorAggregate, can we just foreach ($componentStack as $component)?
Also, can you explain this a little? It's possible that the current component is some other, non-live component? And so we need to look further "up"?
There was a problem hiding this comment.
That's right. If you have for example a, I don't know, Card component as a parent of an Input component, all inside of a Product component, and the input is supposed to control a property of the Product. In HTML this will already be ok, since the product-HTML will have the data-live properties, and the card won't. So, here in PHP, we also need to link it to that Live Component.
I adjusted the comment that was already there.
sneakyvv
force-pushed
the
fix-embedded-component-stack
branch
2 times, most recently
from
58143b3
to
9815f9f
Compare
weaverryan
force-pushed
the
fix-embedded-component-stack
branch
from
9815f9f
to
6933040
Compare
|
Thank you @sneakyvv! |
…hildComponentRenderSubscriber (sneakyvv) This PR was merged into the 2.x branch. Discussion ---------- [LiveComponent] Only consider Live components in InterceptChildComponentRenderSubscriber | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Tickets | | License | MIT This PR is related to the change of #1093. ### Problem The component stack now also contains embedded Components until they are fully rendered, which is good. But, that also means that with this setup ``` {# someTemplate #} <twig:aLiveComponent> <twig:aTwigComponent> <twig:anotherLiveComponent> {# ... #} </twig:anotherLiveComponent> </twig:aTwigComponent> </twig:aLiveComponent> ``` when re-rendering the `anotherLiveComponent` the `InterceptChildComponentRenderSubscriber` would look at the parent component's `childrenFingerprints` to determine if it can short-circuit the rendering process for `anotherLiveComponent` and return an empty html element causing the FE to not do a callback to re-render it. However since that parent is a non-live component it will never short-circuit. ### Solution Only take live components into account. Commits ------- 953721e [LiveComponent] Only consider Live components in InterceptChildComponentRenderSubscriber
Reproduce
(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
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 theComponentRenderfunctions. During the rendering of C, B is already been popped from the stack, and therefore theDataModelPropsSubscriberdoesn'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.