Skip to content

Commit 1301ae8

Browse files
squrioussqurious
committed
Handle field name, no array default sort, enforce type for serializer hydration, prevent regression in PostMountEvent constructor
1 parent b7d6540 commit 1301ae8

17 files changed

+135
-85
lines changed

‎src/LiveComponent/src/Attribute/LiveProp.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ final class LiveProp
6363
*
6464
* Tells if this property should be bound to the URL
6565
*/
66-
private bool $url;
66+
private bool $queryMapping;
6767

6868
/**
6969
* @param bool|array $writable If true, this property can be changed by the frontend.
@@ -80,7 +80,7 @@ final class LiveProp
8080
* from the value used when originally rendering
8181
* this child, the value in the child will be updated
8282
* to match the new value and the child will be re-rendered
83-
* @param bool $url if true, this property will be synchronized with a query parameter
83+
* @param bool $queryMapping if true, this property will be synchronized with a query parameter
8484
* in the URL
8585
*/
8686
public function __construct(
@@ -93,7 +93,7 @@ public function __construct(
9393
string $format = null,
9494
bool $updateFromParent = false,
9595
string|array $onUpdated = null,
96-
bool $url = false,
96+
bool $queryMapping = false,
9797
) {
9898
$this->writable = $writable;
9999
$this->hydrateWith = $hydrateWith;
@@ -104,7 +104,7 @@ public function __construct(
104104
$this->format = $format;
105105
$this->acceptUpdatesFromParent = $updateFromParent;
106106
$this->onUpdated = $onUpdated;
107-
$this->url = $url;
107+
$this->queryMapping = $queryMapping;
108108

109109
if ($this->useSerializerForHydration && ($this->hydrateWith || $this->dehydrateWith)) {
110110
throw new \InvalidArgumentException('Cannot use useSerializerForHydration with hydrateWith or dehydrateWith.');
@@ -200,8 +200,8 @@ public function onUpdated(): null|string|array
200200
return $this->onUpdated;
201201
}
202202

203-
public function url(): bool
203+
public function queryMapping(): bool
204204
{
205-
return $this->url;
205+
return $this->queryMapping;
206206
}
207207
}

‎src/LiveComponent/src/EventListener/QueryStringInitializeSubscriber.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1515
use Symfony\Component\HttpFoundation\RequestStack;
16-
use Symfony\UX\LiveComponent\Metadata\LiveComponentMetadata;
1716
use Symfony\UX\LiveComponent\Metadata\LiveComponentMetadataFactory;
1817
use Symfony\UX\LiveComponent\Util\QueryStringPropsExtractor;
1918
use Symfony\UX\TwigComponent\Event\PreMountEvent;
@@ -43,16 +42,18 @@ public static function getSubscribedEvents(): array
4342

4443
public function onPreMount(PreMountEvent $event): void
4544
{
45+
if (!$event->getMetadata()->get('live', false)) {
46+
// Not a live component
47+
return;
48+
}
49+
4650
$request = $this->requestStack->getMainRequest();
4751

4852
if (null === $request) {
4953
return;
5054
}
5155

52-
$metadata = new LiveComponentMetadata(
53-
$event->getMetadata(),
54-
$this->metadataFactory->createPropMetadatas(new \ReflectionClass($event->getComponent()::class))
55-
);
56+
$metadata = $this->metadataFactory->getMetadata($event->getMetadata()->getName());
5657

5758
if (!$metadata->hasQueryStringBindings()) {
5859
return;

‎src/LiveComponent/src/LiveComponentHydrator.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,8 @@ public function hydrate(object $component, array $props, array $updatedProps, Li
232232
*
233233
* Depending on the prop configuration, the value may be hydrated by a custom method or the Serializer component.
234234
*
235+
* @internal
236+
*
235237
* @throws SerializerExceptionInterface
236238
*/
237239
public function hydrateValue(mixed $value, LivePropMetadata $propMetadata, object $parentObject): mixed
@@ -245,6 +247,10 @@ public function hydrateValue(mixed $value, LivePropMetadata $propMetadata, objec
245247
}
246248

247249
if ($propMetadata->useSerializerForHydration()) {
250+
if (null === $propMetadata->getType()) {
251+
throw new \LogicException(sprintf('The "%s::%s" object should be hydrated with the Serializer, but no type could be guessed.', $parentObject::class, $propMetadata->getName()));
252+
}
253+
248254
return $this->normalizer->denormalize($value, $propMetadata->getType(), 'json', $propMetadata->serializationContext());
249255
}
250256

‎src/LiveComponent/src/Metadata/LiveComponentMetadata.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public function getOnlyPropsThatAcceptUpdatesFromParent(array $inputProps): arra
6565
public function hasQueryStringBindings(): bool
6666
{
6767
foreach ($this->getAllLivePropsMetadata() as $livePropMetadata) {
68-
if ([] !== $livePropMetadata->getQueryStringMapping()) {
68+
if ($livePropMetadata->queryStringMapping()) {
6969
return true;
7070
}
7171
}

‎src/LiveComponent/src/Metadata/LiveComponentMetadataFactory.php

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,14 @@ public function createLivePropMetadata(string $className, string $propertyName,
103103
$isTypeNullable = $type?->allowsNull() ?? true;
104104
}
105105

106-
$queryStringBinding = $this->createQueryStringMapping($propertyName, $liveProp);
107-
108106
return new LivePropMetadata(
109107
$property->getName(),
110108
$liveProp,
111109
$infoType,
112110
$isTypeBuiltIn,
113111
$isTypeNullable,
114112
$collectionValueType,
115-
$queryStringBinding
113+
$liveProp->queryMapping()
116114
);
117115
}
118116

@@ -130,17 +128,6 @@ private static function propertiesFor(\ReflectionClass $class): iterable
130128
}
131129
}
132130

133-
private function createQueryStringMapping(string $propertyName, LiveProp $liveProp): array
134-
{
135-
if (false === $liveProp->url()) {
136-
return [];
137-
}
138-
139-
return [
140-
'name' => $propertyName,
141-
];
142-
}
143-
144131
public function reset(): void
145132
{
146133
$this->liveComponentMetadata = [];

‎src/LiveComponent/src/Metadata/LivePropMetadata.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public function __construct(
3030
private bool $isBuiltIn,
3131
private bool $allowsNull,
3232
private ?Type $collectionValueType,
33-
private array $queryStringMapping = [],
33+
private bool $queryStringMapping,
3434
) {
3535
}
3636

@@ -54,10 +54,7 @@ public function allowsNull(): bool
5454
return $this->allowsNull;
5555
}
5656

57-
/**
58-
* @return array{'name': string}
59-
*/
60-
public function getQueryStringMapping(): array
57+
public function queryStringMapping(): bool
6158
{
6259
return $this->queryStringMapping;
6360
}

‎src/LiveComponent/src/Util/LiveControllerAttributesCreator.php

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,29 @@ public function attributesForRendering(MountedComponent $mounted, ComponentMetad
9898
$mountedAttributes = $mountedAttributes->defaults(['data-live-id' => $id]);
9999
}
100100

101-
if ($isChildComponent) {
102-
$fingerprint = $this->fingerprintCalculator->calculateFingerprint(
103-
$mounted->getInputProps(),
104-
$this->metadataFactory->getMetadata($mounted->getName())
105-
);
106-
if ($fingerprint) {
107-
$attributesCollection->setFingerprint($fingerprint);
108-
}
109-
}
110-
111101
$liveMetadata = $this->metadataFactory->getMetadata($mounted->getName());
112102

113103
if ($liveMetadata->hasQueryStringBindings()) {
114104
$queryMapping = [];
115105
foreach ($liveMetadata->getAllLivePropsMetadata() as $livePropMetadata) {
116-
if ($mapping = $livePropMetadata->getQueryStringMapping()) {
117-
$queryMapping[$livePropMetadata->getName()] = $mapping;
106+
if ($livePropMetadata->queryStringMapping()) {
107+
$frontendName = $livePropMetadata->calculateFieldName($mounted, $livePropMetadata->getName());
108+
$queryMapping[$frontendName] = ['name' => $frontendName];
118109
}
119110
}
120111
$attributesCollection->setQueryUrlMapping($queryMapping);
121112
}
122113

114+
if ($isChildComponent) {
115+
$fingerprint = $this->fingerprintCalculator->calculateFingerprint(
116+
$mounted->getInputProps(),
117+
$liveMetadata
118+
);
119+
if ($fingerprint) {
120+
$attributesCollection->setFingerprint($fingerprint);
121+
}
122+
}
123+
123124
$dehydratedProps = $this->dehydrateComponent(
124125
$mounted->getName(),
125126
$mounted->getComponent(),

‎src/LiveComponent/src/Util/QueryStringPropsExtractor.php

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,10 @@ public function extract(Request $request, LiveComponentMetadata $metadata, objec
4343
$data = [];
4444

4545
foreach ($metadata->getAllLivePropsMetadata() as $livePropMetadata) {
46-
if ($queryStringMapping = $livePropMetadata->getQueryStringMapping()) {
47-
if (null !== ($value = $query[$queryStringMapping['name']] ?? null)) {
48-
if (\is_array($value) && $this->isNumericIndexedArray($value)) {
49-
// Sort numeric array
50-
ksort($value);
51-
} elseif ('' === $value && null !== $livePropMetadata->getType() && (!$livePropMetadata->isBuiltIn() || 'array' === $livePropMetadata->getType())) {
46+
if ($livePropMetadata->queryStringMapping()) {
47+
$frontendName = $livePropMetadata->calculateFieldName($component, $livePropMetadata->getName());
48+
if (null !== ($value = $query[$frontendName] ?? null)) {
49+
if ('' === $value && null !== $livePropMetadata->getType() && (!$livePropMetadata->isBuiltIn() || 'array' === $livePropMetadata->getType())) {
5250
// Cast empty string to empty array for objects and arrays
5351
$value = [];
5452
}
@@ -70,15 +68,14 @@ public function extract(Request $request, LiveComponentMetadata $metadata, objec
7068
return $data;
7169
}
7270

73-
private function isNumericIndexedArray(array $array): bool
74-
{
75-
return 0 === \count(array_filter(array_keys($array), 'is_string'));
76-
}
77-
7871
private function isValueTypeConsistent(mixed $value, LivePropMetadata $livePropMetadata): bool
7972
{
8073
$propType = $livePropMetadata->getType();
8174

75+
if ($livePropMetadata->allowsNull() && null === $value) {
76+
return true;
77+
}
78+
8279
return
8380
\in_array($propType, [null, 'mixed'])
8481
|| $livePropMetadata->isBuiltIn() && ('\is_'.$propType)($value)

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

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
<?php
22

3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
312
namespace Symfony\UX\LiveComponent\Tests\Fixtures\Component;
413

514
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
@@ -10,20 +19,22 @@
1019
#[AsLiveComponent('component_with_url_bound_props')]
1120
class ComponentWithUrlBoundProps
1221
{
13-
#[LiveProp(writable: true, url: true)]
22+
use DefaultActionTrait;
23+
#[LiveProp(queryMapping: true)]
1424
public ?string $prop1 = null;
1525

16-
#[LiveProp(writable: true, url: true)]
26+
#[LiveProp(queryMapping: true)]
1727
public ?int $prop2 = null;
1828

19-
#[LiveProp(writable: true, url: true)]
29+
#[LiveProp(queryMapping: true)]
2030
public array $prop3 = [];
2131

22-
#[LiveProp(writable: true)]
32+
#[LiveProp]
2333
public ?string $prop4 = null;
2434

25-
#[LiveProp(writable: ['address', 'city'], url: true)]
35+
#[LiveProp(queryMapping: true)]
2636
public ?Address $prop5 = null;
2737

28-
use DefaultActionTrait;
38+
#[LiveProp(fieldName: 'field6', queryMapping: true)]
39+
public ?string $prop6 = null;
2940
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,6 @@
33
Prop2: {{ prop2 }}
44
Prop3: {{ prop3|join(',') }}
55
Prop4: {{ prop4 }}
6+
Prop5: address: {{ prop5.address ?? '' }} city: {{ prop5.city ?? '' }}
7+
Prop6: {{ prop6 }}
68
</div>

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ public function testQueryStringMappingAttribute()
149149
'prop2' => ['name' => 'prop2'],
150150
'prop3' => ['name' => 'prop3'],
151151
'prop5' => ['name' => 'prop5'],
152+
'field6' => ['name' => 'field6'],
152153
];
153154

154155
$this->assertEquals($expected, $queryMapping);

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ class QueryStringInitializerSubscriberTest extends KernelTestCase
2121
public function testQueryStringPropsInitialization()
2222
{
2323
$this->browser()
24-
->get('/render-template/render_component_with_url_bound_props?prop1=foo&prop2=42&prop3[]=foo&prop3[]=bar&prop4=unbound')
24+
->get('/render-template/render_component_with_url_bound_props?prop1=foo&prop2=42&prop3[]=foo&prop3[]=bar&prop4=unbound&prop5[address]=foo&prop5[city]=bar&field6=foo')
2525
->assertSuccessful()
2626
->assertContains('Prop1: foo')
2727
->assertContains('Prop2: 42')
2828
->assertContains('Prop3: foo,bar')
2929
->assertContains('Prop4:')
30+
->assertContains('Prop5: address: foo city: bar')
31+
->assertContains('Prop6: foo')
3032
;
3133
}
3234
}

‎src/LiveComponent/tests/Functional/Metadata/LiveComponentMetadataFactoryTest.php

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,16 @@ public function testQueryStringMapping()
3030
$propsMetadataByName[$propMetadata->getName()] = $propMetadata;
3131
}
3232

33-
$this->assertEquals([
34-
'name' => 'prop1',
35-
], $propsMetadataByName['prop1']->getQueryStringMapping());
33+
$this->assertTrue($propsMetadataByName['prop1']->queryStringMapping());
3634

37-
$this->assertEquals([
38-
'name' => 'prop2',
39-
], $propsMetadataByName['prop2']->getQueryStringMapping());
35+
$this->assertTrue($propsMetadataByName['prop2']->queryStringMapping());
4036

41-
$this->assertEquals([
42-
'name' => 'prop3',
43-
], $propsMetadataByName['prop3']->getQueryStringMapping());
37+
$this->assertTrue($propsMetadataByName['prop3']->queryStringMapping());
4438

45-
$this->assertEquals([], $propsMetadataByName['prop4']->getQueryStringMapping());
39+
$this->assertFalse($propsMetadataByName['prop4']->queryStringMapping());
4640

47-
$this->assertEquals([
48-
'name' => 'prop5',
49-
], $propsMetadataByName['prop5']->getQueryStringMapping());
41+
$this->assertTrue($propsMetadataByName['prop5']->queryStringMapping());
42+
43+
$this->assertTrue($propsMetadataByName['prop6']->queryStringMapping());
5044
}
5145
}

‎src/LiveComponent/tests/Functional/Util/QueryStringPropsExtractorTest.php

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,25 @@ public function testExtract(string $queryString, array $expected)
4545
public function getQueryStringTests(): iterable
4646
{
4747
yield from [
48-
['', []],
49-
['prop1=foo', ['prop1' => 'foo']],
50-
['prop2=42', ['prop2' => 42]],
51-
['prop3[]=foo&prop3[]=bar', ['prop3' => ['foo', 'bar']]],
52-
['prop4=foo', []], // not bound
53-
['prop5[address]=foo&prop5[city]=bar', ['prop5' => (function () {
48+
'no query string' => ['', []],
49+
'empty value for nullable string' => ['prop1=', ['prop1' => null]],
50+
'string value' => ['prop1=foo', ['prop1' => 'foo']],
51+
'empty value for nullable int' => ['prop2=', ['prop2' => null]],
52+
'int value' => ['prop2=42', ['prop2' => 42]],
53+
'array value' => ['prop3[]=foo&prop3[]=bar', ['prop3' => ['foo', 'bar']]],
54+
'array value indexed' => ['prop3[1]=foo&prop3[0]=bar', ['prop3' => [1 => 'foo', 0 => 'bar']]],
55+
'not bound prop' => ['prop4=foo', []],
56+
'object value' => ['prop5[address]=foo&prop5[city]=bar', ['prop5' => (function () {
5457
$address = new Address();
5558
$address->address = 'foo';
5659
$address->city = 'bar';
5760

5861
return $address;
5962
})(),
6063
]],
64+
'invalid scalar value' => ['prop1[]=foo&prop1[]=bar', []],
65+
'invalid array value' => ['prop3=foo', []],
66+
'invalid object value' => ['prop5=foo', []],
6167
];
6268
}
6369
}

‎src/LiveComponent/tests/Unit/Metadata/LiveComponentMetadataTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ class LiveComponentMetadataTest extends TestCase
2222
public function testGetOnlyPropsThatAcceptUpdatesFromParent()
2323
{
2424
$propMetadatas = [
25-
new LivePropMetadata('noUpdateFromParent1', new LiveProp(updateFromParent: false), null, false, false, null),
26-
new LivePropMetadata('noUpdateFromParent2', new LiveProp(updateFromParent: false), null, false, false, null),
27-
new LivePropMetadata('yesUpdateFromParent1', new LiveProp(updateFromParent: true), null, false, false, null),
28-
new LivePropMetadata('yesUpdateFromParent2', new LiveProp(updateFromParent: true), null, false, false, null),
25+
new LivePropMetadata('noUpdateFromParent1', new LiveProp(updateFromParent: false), null, false, false, null, false),
26+
new LivePropMetadata('noUpdateFromParent2', new LiveProp(updateFromParent: false), null, false, false, null, false),
27+
new LivePropMetadata('yesUpdateFromParent1', new LiveProp(updateFromParent: true), null, false, false, null, false),
28+
new LivePropMetadata('yesUpdateFromParent2', new LiveProp(updateFromParent: true), null, false, false, null, false),
2929
];
3030
$liveComponentMetadata = new LiveComponentMetadata(new ComponentMetadata([]), $propMetadatas);
3131
$inputProps = [

0 commit comments

Comments
 (0)