Skip to content

Commit 5eb1153

Browse files
KocalKocal
committed
bug #2988 [LiveComponent] Fix new URL calculation with LiveProp using Serializer (and Serializer attributes), and when using custom modifier (Kocal)
This PR was squashed before being merged into the 2.x branch. Discussion ---------- [LiveComponent] Fix new URL calculation with `LiveProp` using Serializer (and Serializer attributes), and when using custom modifier | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Docs? | no <!-- required for new features --> | Issues | Fix #2971, #2991 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT <!-- Replace this notice by a description of your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - For new features, provide some code snippets to help understand usage. - Features and deprecations must be submitted against branch main. - Update/add documentation as required (we can help!) - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> This PR fixes two bugs introduced #2673: 1. See #2971 (comment) for explanations tl;dr: previously, query parameters were updated through `LiveComponentHydrator::dehydrate()` which use the Serializer (if `useSerializerForHydration: true`) and so also used Serializer attributes like `SerializedName`. 2. #2991, when having a `LiveProp(url: new UrlMapping(as: 'foo'))` with a custom modifier that returns a new `LiveProp` with a new `UrlMapping`, ex: ```php public function myModifier(LiveProp $liveProp): LiveProp { $urlMapping = $liveProp->url(); if (!$urlMapping instanceof UrlMapping) { return $liveProp; } return $liveProp->withUrl(new UrlMapping(as: 'alias_' . $urlMapping->as)); } ``` then the prop was persisted as `foo` and not `alias_foo` in the URL. Commits ------- 19fe704 [LiveComponent] Fix new URL calculation when having `#[LiveProp]` with custom `modifier` which returns a new `LiveProp` 07e73bf [LiveComponent] Fix new URL calculation when having `#[LiveProp]` with `useSerializerForHydration: true` and `#[SerializedName]`
2 parents 484f7a6 + 19fe704 commit 5eb1153

File tree

10 files changed

+236
-230
lines changed

10 files changed

+236
-230
lines changed

‎src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ function (ChildDefinition $definition, AsLiveComponent $attribute) {
141141
$container->register('ux.live_component.live_url_subscriber', LiveUrlSubscriber::class)
142142
->setArguments([
143143
new Reference('ux.live_component.metadata_factory'),
144+
new Reference('ux.live_component.component_hydrator'),
144145
new Reference('ux.live_component.url_factory'),
145146
])
146147
->addTag('kernel.event_subscriber')

‎src/LiveComponent/src/EventListener/LiveUrlSubscriber.php

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,12 @@
1212
namespace Symfony\UX\LiveComponent\EventListener;
1313

1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15-
use Symfony\Component\HttpFoundation\Request;
1615
use Symfony\Component\HttpKernel\Event\ResponseEvent;
1716
use Symfony\Component\HttpKernel\KernelEvents;
17+
use Symfony\UX\LiveComponent\LiveComponentHydrator;
1818
use Symfony\UX\LiveComponent\Metadata\LiveComponentMetadataFactory;
1919
use Symfony\UX\LiveComponent\Util\UrlFactory;
20+
use Symfony\UX\TwigComponent\MountedComponent;
2021

2122
/**
2223
* @internal
@@ -27,6 +28,7 @@ class LiveUrlSubscriber implements EventSubscriberInterface
2728

2829
public function __construct(
2930
private LiveComponentMetadataFactory $metadataFactory,
31+
private LiveComponentHydrator $liveComponentHydrator,
3032
private UrlFactory $urlFactory,
3133
) {
3234
}
@@ -42,9 +44,14 @@ public function onKernelResponse(ResponseEvent $event): void
4244
return;
4345
}
4446

47+
if (!$request->attributes->has('_mounted_component')) {
48+
return;
49+
}
50+
4551
$newLiveUrl = null;
4652
if ($previousLiveUrl = $request->headers->get(self::URL_HEADER)) {
47-
$liveProps = $this->getLivePropsFromRequest($request);
53+
$mounted = $request->attributes->get('_mounted_component');
54+
$liveProps = $this->getLiveProps($mounted);
4855
$newLiveUrl = $this->urlFactory->createFromPreviousAndProps($previousLiveUrl, $liveProps['path'], $liveProps['query']);
4956
}
5057

@@ -66,26 +73,26 @@ public static function getSubscribedEvents(): array
6673
* query: array<string, mixed>
6774
* }
6875
*/
69-
private function getLivePropsFromRequest(Request $request): array
76+
private function getLiveProps(MountedComponent $mounted): array
7077
{
71-
$componentName = $request->attributes->get('_live_component');
72-
$metadata = $this->metadataFactory->getMetadata($componentName);
73-
74-
$liveRequestData = $request->attributes->get('_live_request_data') ?? [];
75-
$values = array_merge(
76-
$liveRequestData['props'] ?? [],
77-
$liveRequestData['updated'] ?? [],
78-
$liveRequestData['responseProps'] ?? []
78+
$metadata = $this->metadataFactory->getMetadata($mounted->getName());
79+
80+
$dehydratedProps = $this->liveComponentHydrator->dehydrate(
81+
$mounted->getComponent(),
82+
$mounted->getAttributes(),
83+
$metadata
7984
);
8085

86+
$values = $dehydratedProps->getProps();
87+
8188
$urlLiveProps = [
8289
'path' => [],
8390
'query' => [],
8491
];
85-
foreach ($metadata->getAllUrlMappings() as $name => $urlMapping) {
92+
93+
foreach ($metadata->getAllUrlMappings($mounted->getComponent()) as $name => $urlMapping) {
8694
if (isset($values[$name]) && $urlMapping) {
87-
$urlLiveProps[$urlMapping->mapPath ? 'path' : 'query'][$urlMapping->as ?? $name] =
88-
$values[$name];
95+
$urlLiveProps[$urlMapping->mapPath ? 'path' : 'query'][$urlMapping->as ?? $name] = $values[$name];
8996
}
9097
}
9198

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@ public function getOnlyPropsThatAcceptUpdatesFromParent(array $inputProps): arra
6969
/**
7070
* @return UrlMapping[]
7171
*/
72-
public function getAllUrlMappings(): iterable
72+
public function getAllUrlMappings(object $component): iterable
7373
{
7474
$urlMappings = [];
75-
foreach ($this->livePropsMetadata as $livePropMetadata) {
75+
foreach ($this->getAllLivePropsMetadata($component) as $livePropMetadata) {
7676
if ($livePropMetadata->urlMapping()) {
7777
$urlMappings[$livePropMetadata->getName()] = $livePropMetadata->urlMapping();
7878
}
@@ -81,7 +81,7 @@ public function getAllUrlMappings(): iterable
8181
return $urlMappings;
8282
}
8383

84-
public function hasQueryStringBindings($component): bool
84+
public function hasQueryStringBindings(object $component): bool
8585
{
8686
foreach ($this->getAllLivePropsMetadata($component) as $livePropMetadata) {
8787
if ($livePropMetadata->urlMapping()) {

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace Symfony\UX\LiveComponent\Tests\Fixtures\Component;
1313

1414
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
15+
use Symfony\UX\LiveComponent\Attribute\LiveAction;
16+
use Symfony\UX\LiveComponent\Attribute\LiveArg;
1517
use Symfony\UX\LiveComponent\Attribute\LiveProp;
1618
use Symfony\UX\LiveComponent\DefaultActionTrait;
1719
use Symfony\UX\LiveComponent\Metadata\UrlMapping;
@@ -37,6 +39,9 @@ class ComponentWithUrlBoundProps
3739
#[LiveProp(url: true)]
3840
public ?Address $objectProp = null;
3941

42+
#[LiveProp(url: true, useSerializerForHydration: true)]
43+
public ?Address $objectPropWithSerializerForHydration = null;
44+
4045
#[LiveProp(fieldName: 'field1', url: true)]
4146
public ?string $propWithField1 = null;
4247

@@ -49,6 +54,19 @@ class ComponentWithUrlBoundProps
4954
#[LiveProp]
5055
public ?bool $maybeBoundPropInUrl = false;
5156

57+
#[LiveProp(url: new UrlMapping(as: 'p'), modifier: 'modifyPropertyWithModifierAndAlias')]
58+
public ?string $propertyWithModifierAndAlias = null;
59+
60+
public function modifyPropertyWithModifierAndAlias(LiveProp $liveProp): LiveProp
61+
{
62+
$urlMapping = $liveProp->url();
63+
if (!$urlMapping instanceof UrlMapping) {
64+
return $liveProp;
65+
}
66+
67+
return $liveProp->withUrl(new UrlMapping(as: 'alias_' . $urlMapping->as));
68+
}
69+
5270
public function getField2(): string
5371
{
5472
return 'field2';
@@ -83,5 +101,13 @@ public function modifyBoundPropWithCustomAlias(LiveProp $liveProp): LiveProp
83101
return $liveProp;
84102
}
85103

104+
#[LiveAction]
105+
public function updateLiveProp(#[LiveArg] string $propName, #[LiveArg] mixed $propValue): void
106+
{
107+
if (!property_exists($this, $propName)) {
108+
throw new \InvalidArgumentException(\sprintf('Property "%s" does not exist on component "%s".', $propName, static::class));
109+
}
86110

111+
$this->{$propName} = $propValue;
112+
}
87113
}

‎src/LiveComponent/tests/Fixtures/Dto/Address.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@
22

33
namespace Symfony\UX\LiveComponent\Tests\Fixtures\Dto;
44

5+
use Symfony\Component\Serializer\Attribute\SerializedName;
6+
57
class Address
68
{
79
public string $address;
10+
11+
#[SerializedName(serializedName: 'c')]
812
public string $city;
9-
}
13+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ public function testQueryStringMappingAttribute()
141141
'boundPropWithCustomAlias' => ['name' => 'customAlias'],
142142
'pathProp' => ['name' => 'pathProp'],
143143
'pathPropWithAlias' => ['name' => 'pathAlias'],
144+
'objectPropWithSerializerForHydration' => ['name' => 'objectPropWithSerializerForHydration'],
145+
'propertyWithModifierAndAlias' => ['name' => 'alias_p'],
144146
];
145147

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

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

Lines changed: 80 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\UX\LiveComponent\Tests\Functional\EventListener;
1313

1414
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
15+
use Symfony\UX\LiveComponent\Tests\Fixtures\Dto\Address;
1516
use Symfony\UX\LiveComponent\Tests\LiveComponentTestHelper;
1617
use Zenstruck\Browser\Test\HasBrowser;
1718

@@ -26,17 +27,20 @@ public function getTestData(): iterable
2627
'previousLocation' => null,
2728
'expectedLocation' => null,
2829
'props' => [],
30+
'args' => [],
2931
];
3032
yield 'unknown_previous_location' => [
3133
'previousLocation' => 'foo/bar',
3234
'expectedLocation' => null,
3335
'props' => [],
36+
'args' => [],
3437
];
3538

3639
yield 'no_prop' => [
3740
'previousLocation' => '/route_with_prop/foo',
3841
'expectedLocation' => null,
3942
'props' => [],
43+
'args' => [],
4044
];
4145

4246
yield 'no_change' => [
@@ -45,52 +49,118 @@ public function getTestData(): iterable
4549
'props' => [
4650
'pathProp' => 'foo',
4751
],
52+
'args' => [],
4853
];
4954

50-
yield 'prop_changed' => [
55+
yield 'path_prop_changed' => [
5156
'previousLocation' => '/route_with_prop/foo',
5257
'expectedLocation' => '/route_with_prop/bar',
5358
'props' => [
5459
'pathProp' => 'foo',
5560
],
56-
'updated' => [
57-
'pathProp' => 'bar',
61+
'args' => [
62+
'propName' => 'pathProp',
63+
'propValue' => 'bar',
5864
],
5965
];
6066

61-
yield 'alias_prop_changed' => [
67+
yield 'path_alias_prop_changed' => [
6268
'previousLocation' => '/route_with_alias_prop/foo',
6369
'expectedLocation' => '/route_with_alias_prop/bar',
6470
'props' => [
6571
'pathPropWithAlias' => 'foo',
6672
],
67-
'updated' => [
68-
'pathPropWithAlias' => 'bar',
73+
'args' => [
74+
'propName' => 'pathPropWithAlias',
75+
'propValue' => 'bar',
6976
],
7077
];
78+
79+
yield 'query_alias_prop_changed' => [
80+
'previousLocation' => '/',
81+
'expectedLocation' => '/?q=search%2Bterm',
82+
'props' => [],
83+
'args' => [
84+
'propName' => 'boundPropWithAlias',
85+
'propValue' => 'search+term',
86+
],
87+
];
88+
89+
yield 'path_and_query_alias_prop_changed' => [
90+
'previousLocation' => '/route_with_prop/foo',
91+
'expectedLocation' => '/route_with_prop/baz?q=foo+bar',
92+
'props' => [
93+
'pathProp' => 'baz',
94+
],
95+
'args' => [
96+
'propName' => 'boundPropWithAlias',
97+
'propValue' => 'foo bar',
98+
],
99+
];
100+
101+
$address = new Address();
102+
$address->address = '123 Main St';
103+
$address->city = 'Anytown';
104+
yield 'with an object in query, keys "address" and "city" must be present' => [
105+
'previousLocation' => '/',
106+
'expectedLocation' => '/?objectProp%5Baddress%5D=123+Main+St&objectProp%5Bcity%5D=Anytown&q=search',
107+
'props' => [
108+
'objectProp' => $address,
109+
],
110+
'args' => [
111+
'propName' => 'boundPropWithAlias',
112+
'propValue' => 'search',
113+
],
114+
];
115+
116+
$address = new Address();
117+
$address->address = '123 Main St';
118+
$address->city = 'Anytown';
119+
yield 'with an object in query, with "useSerializerForHydration: true", keys "address" and "c" must be present' => [
120+
'previousLocation' => '/',
121+
'expectedLocation' => '/?intProp=3&objectPropWithSerializerForHydration%5Baddress%5D=123+Main+St&objectPropWithSerializerForHydration%5Bc%5D=Anytown',
122+
'props' => [
123+
'objectPropWithSerializerForHydration' => $address,
124+
],
125+
'args' => [
126+
'propName' => 'intProp',
127+
'propValue' => '3',
128+
],
129+
];
130+
131+
yield 'query with alias ("p") and modifier (prefix by "alias_")' => [
132+
'previousLocation' => '/',
133+
'expectedLocation' => '/?alias_p=test',
134+
'props' => [
135+
'propertyWithModifierAndAlias' => 'test',
136+
],
137+
'args' => [],
138+
];
71139
}
72140

73141
/**
74142
* @dataProvider getTestData
75143
*/
76-
public function testNoHeader(
144+
public function testNewLiveUrlAfterLiveAction(
77145
?string $previousLocation,
78146
?string $expectedLocation,
79147
array $props,
80-
array $updated = [],
148+
array $args,
81149
): void {
82150
$component = $this->mountComponent('component_with_url_bound_props', $props);
83151
$dehydrated = $this->dehydrateComponent($component);
84152

85153
$this->browser()
86154
->throwExceptions()
87155
->post(
88-
'/_components/component_with_url_bound_props',
156+
[] === $args
157+
? '/_components/component_with_url_bound_props'
158+
: '/_components/component_with_url_bound_props/updateLiveProp',
89159
[
90160
'body' => [
91161
'data' => json_encode([
92162
'props' => $dehydrated->getProps(),
93-
'updated' => $updated,
163+
'args' => $args,
94164
]),
95165
],
96166
'headers' => [

0 commit comments

Comments
 (0)