Skip to content

Commit 22c0fa8

Browse files
weaverryanweaverryan
committed
bug #596 [Live] Various bug fixes for new 2.5 parent-child fingerprint calculation (weaverryan)
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [Live] Various bug fixes for new 2.5 parent-child fingerprint calculation | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | Tickets | Fix #540 | License | MIT Hi! Version 2.5 included a big upgrade to parent-child components. Not surprisingly, there were some bugs. This fixes 4 of those - thanks to the excellent reproducer in #540. Cheers! Commits ------- 9c870b4 [Live] Fixing a bug where parent component model would be used to set child model field 6040306 [Live] Fixing completely incorrect attribute name for fingerprint 9f313bc [Live] fixing bug where custom data-live-id would be ignored in re-render 2e17b4d [Live] Fixing a bug so that data-live-id is not overridden for child components
2 parents 0e22b90 + 9c870b4 commit 22c0fa8

File tree

11 files changed

+134
-27
lines changed

11 files changed

+134
-27
lines changed

‎src/LiveComponent/assets/dist/live_controller.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,6 +2112,9 @@ class SetValueOntoModelFieldsPlugin {
21122112
if (element instanceof HTMLFormElement) {
21132113
return;
21142114
}
2115+
if (!elementBelongsToThisComponent(element, component)) {
2116+
return;
2117+
}
21152118
const modelDirective = getModelDirectiveFromElement(element);
21162119
if (!modelDirective) {
21172120
return;

‎src/LiveComponent/assets/src/Component/plugins/SetValueOntoModelFieldsPlugin.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import Component from '../index';
22
import {
3+
elementBelongsToThisComponent,
34
getModelDirectiveFromElement,
45
getValueFromElement,
56
setValueOnElement
@@ -38,6 +39,10 @@ export default class implements PluginInterface {
3839
return;
3940
}
4041

42+
if (!elementBelongsToThisComponent(element, component)) {
43+
return;
44+
}
45+
4146
const modelDirective = getModelDirectiveFromElement(element);
4247
if (!modelDirective) {
4348
return;

‎src/LiveComponent/assets/test/controller/model.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,45 @@ describe('LiveController data-model Tests', () => {
628628
expect(commentField.value).toEqual('MMMM SO GOOD');
629629
});
630630

631+
it('does not try to set the value of inputs inside a child component', async () => {
632+
const test = await createTest({ comment: 'cookie', childComment: 'mmmm' }, (data: any) => `
633+
<div ${initComponent(data)}>
634+
<textarea data-model="comment" id="parent-comment"></textarea>
635+
636+
<div ${initComponent({ comment: data.childComment }, {}, {id: 'the-child-id'})}>
637+
<textarea data-model="comment" id="child-comment"></textarea>
638+
</div>
639+
</div>
640+
`);
641+
642+
const commentField = test.element.querySelector('#parent-comment');
643+
if (!(commentField instanceof HTMLTextAreaElement)) {
644+
throw new Error('wrong type');
645+
}
646+
expect(commentField.value).toEqual('cookie');
647+
648+
const childCommentField = test.element.querySelector('#child-comment');
649+
if (!(childCommentField instanceof HTMLTextAreaElement)) {
650+
throw new Error('wrong type');
651+
}
652+
expect(childCommentField.value).toEqual('mmmm');
653+
654+
// NOW we will re-render
655+
test.expectsAjaxCall('get')
656+
.expectSentData(test.initialData)
657+
// change the data to be extra tricky
658+
.serverWillChangeData((data) => {
659+
data.comment = 'i like apples';
660+
})
661+
.init();
662+
663+
await test.component.render();
664+
665+
expect(commentField.value).toEqual('i like apples');
666+
// child component should not have been re-rendered
667+
expect(childCommentField.value).toEqual('mmmm');
668+
});
669+
631670
it('keeps the unsynced value of an input on re-render, but accepts other changes to the field', async () => {
632671
const test = await createTest({
633672
comment: 'Live components',

‎src/LiveComponent/src/EventListener/AddLiveAttributesSubscriber.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,13 @@ private function getLiveAttributes(MountedComponent $mounted, ComponentMetadata
112112
}
113113

114114
if ($this->container->get(ComponentStack::class)->hasParentComponent()) {
115-
$id = $this->container->get(DeterministicTwigIdCalculator::class)->calculateDeterministicId();
116-
$attributes['data-live-id'] = $helper->escapeAttribute($id);
115+
if (!isset($mounted->getAttributes()->all()['data-live-id'])) {
116+
$id = $this->container->get(DeterministicTwigIdCalculator::class)->calculateDeterministicId();
117+
$attributes['data-live-id'] = $helper->escapeAttribute($id);
118+
}
117119

118120
$fingerprint = $this->container->get(FingerprintCalculator::class)->calculateFingerprint($mounted->getInputProps());
119-
$attributes['data-live-value-fingerprint'] = $helper->escapeAttribute($fingerprint);
121+
$attributes['data-live-fingerprint-value'] = $helper->escapeAttribute($fingerprint);
120122
}
121123

122124
return new ComponentAttributes($attributes);

‎src/LiveComponent/src/EventListener/InterceptChildComponentRenderSubscriber.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function preComponentCreated(PreCreateForRenderEvent $event): void
5959
$childFingerprints = $parentComponent->getExtraMetadata(self::CHILDREN_FINGERPRINTS_METADATA_KEY);
6060

6161
// get the deterministic id for this child, but without incrementing the counter yet
62-
$deterministicId = $this->deterministicTwigIdCalculator->calculateDeterministicId(increment: false);
62+
$deterministicId = $event->getProps()['data-live-id'] ?? $this->deterministicTwigIdCalculator->calculateDeterministicId(increment: false);
6363
if (!isset($childFingerprints[$deterministicId])) {
6464
// child fingerprint wasn't set, it is likely a new child, allow it to render fully
6565
return;

‎src/LiveComponent/tests/Fixtures/Component/TodoListComponent.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,8 @@ final class TodoListComponent
2424
#[LiveProp]
2525
public array $items = [];
2626

27+
#[LiveProp(writable: true)]
28+
public $includeDataLiveId = false;
29+
2730
use DefaultActionTrait;
2831
}

‎src/LiveComponent/tests/Fixtures/templates/components/todo_list.html.twig

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33

44
<ul>
55
{% for item in items %}
6-
{{ component('todo_item', {
7-
text: item.text
8-
}) }}
6+
{% set componentProps = { text: item.text } %}
7+
{% if includeDataLiveId %}
8+
{% set componentProps = componentProps|merge({'data-live-id': ('todo-item-' ~ loop.index) }) %}
9+
{% endif %}
10+
{{ component('todo_item', componentProps) }}
911
{% endfor %}
1012
</ul>
1113
</div>

‎src/LiveComponent/tests/Fixtures/templates/render_todo_list_with_live_id.html.twig

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{{ component('todo_list', {
2+
items: [
3+
{ text: 'milk'},
4+
{ text: 'cheese'},
5+
{ text: 'milk'},
6+
],
7+
includeDataLiveId: true
8+
}) }}

‎src/LiveComponent/tests/Functional/EventListener/AddLiveAttributesSubscriberTest.php

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@
2020
final class AddLiveAttributesSubscriberTest extends KernelTestCase
2121
{
2222
use HasBrowser;
23+
/**
24+
* The deterministic id of the "todo_item" components in todo_list.html.twig.
25+
* If that template changes, this will need to be updated.
26+
*/
27+
public const TODO_ITEM_DETERMINISTIC_PREFIX = 'live-289310975-';
2328

2429
public function testInitLiveComponent(): void
2530
{
@@ -85,14 +90,29 @@ public function testItAddsIdAndFingerprintToChildComponent(): void
8590

8691
$lis = $ul->children('li');
8792
// deterministic id: should not change, and counter should increase
88-
$this->assertSame('live-3649730296-0', $lis->first()->attr('data-live-id'));
89-
$this->assertSame('live-3649730296-2', $lis->last()->attr('data-live-id'));
93+
$this->assertSame(self::TODO_ITEM_DETERMINISTIC_PREFIX.'0', $lis->first()->attr('data-live-id'));
94+
$this->assertSame(self::TODO_ITEM_DETERMINISTIC_PREFIX.'2', $lis->last()->attr('data-live-id'));
9095

9196
// fingerprints
9297
// first and last both have the same input - thus fingerprint
93-
$this->assertSame('sH/Rwn0x37n3KyMWQLa6OBPgglriBZqlwPLnm/EQTlE=', $lis->first()->attr('data-live-value-fingerprint'));
94-
$this->assertSame('sH/Rwn0x37n3KyMWQLa6OBPgglriBZqlwPLnm/EQTlE=', $lis->last()->attr('data-live-value-fingerprint'));
98+
$this->assertSame('sH/Rwn0x37n3KyMWQLa6OBPgglriBZqlwPLnm/EQTlE=', $lis->first()->attr('data-live-fingerprint-value'));
99+
$this->assertSame('sH/Rwn0x37n3KyMWQLa6OBPgglriBZqlwPLnm/EQTlE=', $lis->last()->attr('data-live-fingerprint-value'));
95100
// middle has a different fingerprint
96-
$this->assertSame('cuOKkrHC9lOmBa6dyVZ3S0REdw4CKCwJgLDdrVoTb2g=', $lis->eq(1)->attr('data-live-value-fingerprint'));
101+
$this->assertSame('cuOKkrHC9lOmBa6dyVZ3S0REdw4CKCwJgLDdrVoTb2g=', $lis->eq(1)->attr('data-live-fingerprint-value'));
102+
}
103+
104+
public function testItDoesNotOverrideDataLiveIdIfSpecified(): void
105+
{
106+
$ul = $this->browser()
107+
->visit('/render-template/render_todo_list_with_live_id')
108+
->assertSuccessful()
109+
->crawler()
110+
->filter('ul')
111+
;
112+
113+
$lis = $ul->children('li');
114+
// deterministic id: is not used: data-live-id was passed in manually
115+
$this->assertSame('todo-item-1', $lis->first()->attr('data-live-id'));
116+
$this->assertSame('todo-item-3', $lis->last()->attr('data-live-id'));
97117
}
98118
}

‎src/LiveComponent/tests/Functional/EventListener/InterceptChildComponentRenderSubscriberTest.php

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ final class InterceptChildComponentRenderSubscriberTest extends KernelTestCase
2525
// if you pass in 3 "items" with data that matches what's used by default
2626
// in buildUrlForTodoListComponent
2727
private static array $actualTodoItemFingerprints = [
28-
'live-3649730296-0' => 'LwqODySoRx3q+v64EzalGouzpSHWKIm0jENTUGtQloE=',
29-
'live-3649730296-1' => 'gn9PcPUqL0tkeLSw0ZuhOj96dwIpiBmJPoO5NPync2o=',
30-
'live-3649730296-2' => 'ndV00y/qOSH11bjOKGDJVRsxANtbudYB6K8D46viUI8=',
28+
AddLiveAttributesSubscriberTest::TODO_ITEM_DETERMINISTIC_PREFIX.'0' => 'LwqODySoRx3q+v64EzalGouzpSHWKIm0jENTUGtQloE=',
29+
AddLiveAttributesSubscriberTest::TODO_ITEM_DETERMINISTIC_PREFIX.'1' => 'gn9PcPUqL0tkeLSw0ZuhOj96dwIpiBmJPoO5NPync2o=',
30+
AddLiveAttributesSubscriberTest::TODO_ITEM_DETERMINISTIC_PREFIX.'2' => 'ndV00y/qOSH11bjOKGDJVRsxANtbudYB6K8D46viUI8=',
3131
];
3232

3333
public function testItAllowsFullChildRenderOnMissingFingerprints(): void
@@ -56,10 +56,30 @@ public function testItRendersEmptyElementOnMatchingFingerprint(): void
5656
;
5757
}
5858

59+
public function testItRendersEmptyElementOnMatchingFingerprintWithCustomDataLiveId(): void
60+
{
61+
$fingerPrintsWithCustomLiveId = [];
62+
foreach (array_values(self::$actualTodoItemFingerprints) as $key => $fingerprintValue) {
63+
// creating fingerprints to match todo_list.html.twig
64+
$fingerPrintsWithCustomLiveId['todo-item-'.$key + 1] = $fingerprintValue;
65+
}
66+
67+
$this->browser()
68+
->visit($this->buildUrlForTodoListComponent($fingerPrintsWithCustomLiveId, true))
69+
->assertSuccessful()
70+
->assertHtml()
71+
// no lis (because we render a div always)
72+
->assertElementCount('ul li', 0)
73+
// because we actually slip in a div element
74+
->assertElementCount('ul div', 3)
75+
->assertNotContains('todo item')
76+
;
77+
}
78+
5979
public function testItRendersNewPropWhenFingerprintDoesNotMatch(): void
6080
{
6181
$fingerprints = self::$actualTodoItemFingerprints;
62-
$fingerprints['live-3649730296-1'] = 'wrong fingerprint';
82+
$fingerprints[AddLiveAttributesSubscriberTest::TODO_ITEM_DETERMINISTIC_PREFIX.'1'] = 'wrong fingerprint';
6383

6484
$this->browser()
6585
->visit($this->buildUrlForTodoListComponent($fingerprints))
@@ -74,26 +94,30 @@ public function testItRendersNewPropWhenFingerprintDoesNotMatch(): void
7494

7595
// 1st and 3rd render empty
7696
// fingerprint changed in 2nd, so it renders new fingerprint + props
77-
$this->assertStringContainsString(<<<EOF
78-
<ul>
79-
<div data-live-id="live-3649730296-0"></div>
80-
<div data-live-id="live-3649730296-1" data-live-fingerprint-value="gn9PcPUqL0tkeLSw0ZuhOj96dwIpiBmJPoO5NPync2o&#x3D;" data-live-props-value="&#x7B;&quot;_checksum&quot;&#x3A;&quot;YrPg4mHsB82fR&#x5C;&#x2F;VmTL3gJIX32kS8HvWy&#x5C;&#x2F;9uKSs&#x5C;&#x2F;aPfk&#x3D;&quot;&#x7D;"></div>
81-
<div data-live-id="live-3649730296-2"></div>
82-
</ul>
83-
EOF,
84-
$content);
85-
})
86-
;
97+
$this->assertStringContainsString(sprintf(
98+
'<div data-live-id="%s0"></div>',
99+
AddLiveAttributesSubscriberTest::TODO_ITEM_DETERMINISTIC_PREFIX
100+
), $content);
101+
$this->assertStringContainsString(sprintf(
102+
'<div data-live-id="%s1" data-live-fingerprint-value="gn9PcPUqL0tkeLSw0ZuhOj96dwIpiBmJPoO5NPync2o&#x3D;" data-live-props-value="&#x7B;&quot;_checksum&quot;&#x3A;&quot;YrPg4mHsB82fR&#x5C;&#x2F;VmTL3gJIX32kS8HvWy&#x5C;&#x2F;9uKSs&#x5C;&#x2F;aPfk&#x3D;&quot;&#x7D;"></div>',
103+
AddLiveAttributesSubscriberTest::TODO_ITEM_DETERMINISTIC_PREFIX
104+
), $content);
105+
$this->assertStringContainsString(sprintf(
106+
'<div data-live-id="%s2"></div>',
107+
AddLiveAttributesSubscriberTest::TODO_ITEM_DETERMINISTIC_PREFIX
108+
), $content);
109+
});
87110
}
88111

89-
private function buildUrlForTodoListComponent(array $childrenFingerprints): string
112+
private function buildUrlForTodoListComponent(array $childrenFingerprints, bool $includeLiveId = false): string
90113
{
91114
$component = $this->mountComponent('todo_list', [
92115
'items' => [
93116
['text' => 'wake up'],
94117
['text' => 'high five a friend'],
95118
['text' => 'take a nap'],
96119
],
120+
'includeDataLiveId' => $includeLiveId,
97121
]);
98122

99123
$dehydrated = $this->dehydrateComponent($component);

‎ux.symfony.com/assets/bootstrap.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { startStimulusApp } from '@symfony/stimulus-bridge';
22
import Clipboard from 'stimulus-clipboard'
3+
import Live from '../live_components';
34

45
// Registers Stimulus controllers from controllers.json and in the controllers/ directory
56
export const app = startStimulusApp(require.context(

0 commit comments

Comments
 (0)