Skip to content

Commit 701576e

Browse files
KocalKocal
committed
bug #3053 [LiveComponent] Refactor and fix some edgecases on new URL generation with path + query LiveProps (Kocal)
This PR was merged into the 2.x branch. Discussion ---------- [LiveComponent] Refactor and fix some edgecases on new URL generation with path + query LiveProps | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Docs? | no <!-- required for new features --> | Issues | Fix #2987, fix #3015, fix #3052 <!-- 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 aims to fix the issues #2987, #3015, and #3052, by rewriting the logic to compute the new `X-Live-Url` when using `LiveProp()` (either with path paremeters or query parameters), inspired by `@smnandre`'s work Commits ------- 5c1441f [LiveComponent] Refactor and fix (again) the new URL (with path and query LiveProps) generation
2 parents 126a80b + 5c1441f commit 701576e

File tree

9 files changed

+166
-388
lines changed

9 files changed

+166
-388
lines changed

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
use Symfony\UX\LiveComponent\Util\LiveControllerAttributesCreator;
5454
use Symfony\UX\LiveComponent\Util\RequestPropsExtractor;
5555
use Symfony\UX\LiveComponent\Util\TwigAttributeHelperFactory;
56-
use Symfony\UX\LiveComponent\Util\UrlFactory;
5756
use Symfony\UX\TwigComponent\ComponentFactory;
5857
use Symfony\UX\TwigComponent\ComponentRenderer;
5958

@@ -142,7 +141,7 @@ function (ChildDefinition $definition, AsLiveComponent $attribute) {
142141
->setArguments([
143142
new Reference('ux.live_component.metadata_factory'),
144143
new Reference('ux.live_component.component_hydrator'),
145-
new Reference('ux.live_component.url_factory'),
144+
new Reference('router'),
146145
])
147146
->addTag('kernel.event_subscriber')
148147
;
@@ -213,9 +212,6 @@ function (ChildDefinition $definition, AsLiveComponent $attribute) {
213212

214213
$container->register('ux.live_component.attribute_helper_factory', TwigAttributeHelperFactory::class);
215214

216-
$container->register('ux.live_component.url_factory', UrlFactory::class)
217-
->setArguments([new Reference('router')]);
218-
219215
$container->register('ux.live_component.live_controller_attributes_creator', LiveControllerAttributesCreator::class)
220216
->setArguments([
221217
new Reference('ux.live_component.metadata_factory'),

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

Lines changed: 72 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1515
use Symfony\Component\HttpKernel\Event\ResponseEvent;
1616
use Symfony\Component\HttpKernel\KernelEvents;
17+
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
18+
use Symfony\Component\Routing\RouterInterface;
1719
use Symfony\UX\LiveComponent\LiveComponentHydrator;
1820
use Symfony\UX\LiveComponent\Metadata\LiveComponentMetadataFactory;
19-
use Symfony\UX\LiveComponent\Util\UrlFactory;
2021
use Symfony\UX\TwigComponent\MountedComponent;
2122

2223
/**
@@ -29,35 +30,28 @@ class LiveUrlSubscriber implements EventSubscriberInterface
2930
public function __construct(
3031
private LiveComponentMetadataFactory $metadataFactory,
3132
private LiveComponentHydrator $liveComponentHydrator,
32-
private UrlFactory $urlFactory,
33+
private RouterInterface $router,
3334
) {
3435
}
3536

3637
public function onKernelResponse(ResponseEvent $event): void
3738
{
38-
if (!$event->isMainRequest()) {
39-
return;
40-
}
41-
4239
$request = $event->getRequest();
43-
if (!$request->attributes->has('_live_component')) {
40+
if (!$event->isMainRequest()
41+
|| !$event->getResponse()->isSuccessful()
42+
|| !$request->attributes->has('_live_component')
43+
|| !$request->attributes->has('_mounted_component')
44+
|| !($previousLiveUrl = $request->headers->get(self::URL_HEADER))
45+
) {
4446
return;
4547
}
4648

47-
if (!$request->attributes->has('_mounted_component')) {
48-
return;
49-
}
49+
/** @var MountedComponent $mounted */
50+
$mounted = $request->attributes->get('_mounted_component');
5051

51-
$newLiveUrl = null;
52-
if ($previousLiveUrl = $request->headers->get(self::URL_HEADER)) {
53-
$mounted = $request->attributes->get('_mounted_component');
54-
$liveProps = $this->getLiveProps($mounted);
55-
$newLiveUrl = $this->urlFactory->createFromPreviousAndProps($previousLiveUrl, $liveProps['path'], $liveProps['query']);
56-
}
52+
[$pathProps, $queryProps] = $this->extractUrlLiveProps($mounted);
5753

58-
if ($newLiveUrl) {
59-
$event->getResponse()->headers->set(self::URL_HEADER, $newLiveUrl);
60-
}
54+
$event->getResponse()->headers->set(self::URL_HEADER, $this->generateNewLiveUrl($previousLiveUrl, $pathProps, $queryProps));
6155
}
6256

6357
public static function getSubscribedEvents(): array
@@ -68,34 +62,73 @@ public static function getSubscribedEvents(): array
6862
}
6963

7064
/**
71-
* @return array{
72-
* path: array<string, mixed>,
73-
* query: array<string, mixed>
74-
* }
65+
* @return array{ array<string, mixed>, array<string, mixed> }
7566
*/
76-
private function getLiveProps(MountedComponent $mounted): array
67+
private function extractUrlLiveProps(MountedComponent $mounted): array
7768
{
78-
$metadata = $this->metadataFactory->getMetadata($mounted->getName());
69+
$pathProps = $queryProps = [];
70+
71+
$mountedMetadata = $this->metadataFactory->getMetadata($mounted->getName());
72+
73+
if ([] !== $urlMappings = $mountedMetadata->getAllUrlMappings($mounted->getComponent())) {
74+
$dehydratedProps = $this->liveComponentHydrator->dehydrate($mounted->getComponent(), $mounted->getAttributes(), $mountedMetadata);
75+
$props = $dehydratedProps->getProps();
76+
77+
foreach ($urlMappings as $name => $urlMapping) {
78+
if (\array_key_exists($name, $props)) {
79+
if ($urlMapping->mapPath) {
80+
$pathProps[$urlMapping->as ?? $name] = $props[$name];
81+
} else {
82+
$queryProps[$urlMapping->as ?? $name] = $props[$name];
83+
}
84+
}
85+
}
86+
}
7987

80-
$dehydratedProps = $this->liveComponentHydrator->dehydrate(
81-
$mounted->getComponent(),
82-
$mounted->getAttributes(),
83-
$metadata
84-
);
88+
return [$pathProps, $queryProps];
89+
}
8590

86-
$values = $dehydratedProps->getProps();
91+
private function generateNewLiveUrl(string $previousUrl, array $pathProps, array $queryProps): string
92+
{
93+
$previousUrlParsed = parse_url($previousUrl);
94+
$newUrl = $previousUrlParsed['path'];
95+
$newQueryString = $previousUrlParsed['query'] ?? '';
96+
97+
if ([] !== $pathProps) {
98+
$context = $this->router->getContext();
99+
try {
100+
// Re-create a context for the URL rendering the current LiveComponent
101+
$tmpContext = clone $context;
102+
$tmpContext->setMethod('GET');
103+
$this->router->setContext($tmpContext);
104+
105+
$routeMatched = $this->router->match($previousUrlParsed['path']);
106+
$routeParams = [];
107+
foreach ($routeMatched as $k => $v) {
108+
if ('_route' === $k || '_controller' === $k) {
109+
continue;
110+
}
111+
$routeParams[$k] = \array_key_exists($k, $pathProps) ? $pathProps[$k] : $v;
112+
}
113+
114+
$newUrl = $this->router->generate($routeMatched['_route'], $routeParams);
115+
} catch (ResourceNotFoundException) {
116+
// reuse the previous URL path
117+
} finally {
118+
$this->router->setContext($context);
119+
}
120+
}
87121

88-
$urlLiveProps = [
89-
'path' => [],
90-
'query' => [],
91-
];
122+
if ([] !== $queryProps) {
123+
$previousQueryString = [];
92124

93-
foreach ($metadata->getAllUrlMappings($mounted->getComponent()) as $name => $urlMapping) {
94-
if (isset($values[$name]) && $urlMapping) {
95-
$urlLiveProps[$urlMapping->mapPath ? 'path' : 'query'][$urlMapping->as ?? $name] = $values[$name];
125+
if (isset($previousUrlParsed['query'])) {
126+
parse_str($previousUrlParsed['query'], $previousQueryString);
96127
}
128+
129+
$newQueryString = http_build_query([...$previousQueryString, ...$queryProps]);
97130
}
98131

99-
return $urlLiveProps;
132+
return $newUrl.($newQueryString ? '?'.$newQueryString : '');
100133
}
101134
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,10 @@ public function getOnlyPropsThatAcceptUpdatesFromParent(array $inputProps): arra
6969
/**
7070
* @return UrlMapping[]
7171
*/
72-
public function getAllUrlMappings(object $component): iterable
72+
public function getAllUrlMappings(object $component): array
7373
{
7474
$urlMappings = [];
75+
7576
foreach ($this->getAllLivePropsMetadata($component) as $livePropMetadata) {
7677
if ($livePropMetadata->urlMapping()) {
7778
$urlMappings[$livePropMetadata->getName()] = $livePropMetadata->urlMapping();

‎src/LiveComponent/src/Util/UrlFactory.php

Lines changed: 0 additions & 108 deletions
This file was deleted.

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,9 @@ public function modifyMaybeBoundProp(LiveProp $prop): LiveProp
9292
#[LiveProp(writable: true, url: new UrlMapping(as: 'pathAlias', mapPath: true))]
9393
public ?string $pathPropWithAlias = null;
9494

95+
#[LiveProp(writable: true, url: new UrlMapping(mapPath: true))]
96+
public ?string $pathPropForAnotherController = 'foo';
97+
9598
public function modifyBoundPropWithCustomAlias(LiveProp $liveProp): LiveProp
9699
{
97100
if ($this->customAlias) {

‎src/LiveComponent/tests/Fixtures/Kernel.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,9 @@ protected function configureRoutes(RoutingConfigurator $routes): void
229229
$routes->add('homepage', '/')->controller('kernel::index');
230230
$routes->add('alternate_live_route', '/alt/{_live_component}/{_live_action}')->defaults(['_live_action' => 'get']);
231231
$routes->add('localized_route', '/locale/{_locale}/{_live_component}/{_live_action}')->defaults(['_live_action' => 'get']);
232-
$routes->add('route_with_prop', '/route_with_prop/{pathProp}')->methods(['GET']);
233-
$routes->add('route_with_alias_prop', '/route_with_alias_prop/{pathAlias}');
232+
$routes->add('route_with_prop', '/route_with_prop/{pathProp}')->methods(['GET'])->requirements(['pathProp' => '\w+']);
233+
$routes->add('route_with_alias_prop', '/route_with_alias_prop/{pathAlias}')->requirements(['pathAlias' => '\w+']);
234+
$routes->add('route_with_two_props', '/route_with_two_props/{pathProp}/{pathAlias}')->methods(['GET'])->requirements(['pathProp' => '\w+', 'pathAlias' => '\w+']);
235+
$routes->add('route_with_two_path_params_but_one_prop', '/route_with_two_path_params_but_one_prop/{pathProp}/{id}')->methods(['GET'])->requirements(['pathProp' => '\w+', 'id' => '\d+']);
234236
}
235237
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ public function testQueryStringMappingAttribute()
143143
'pathPropWithAlias' => ['name' => 'pathAlias'],
144144
'objectPropWithSerializerForHydration' => ['name' => 'objectPropWithSerializerForHydration'],
145145
'propertyWithModifierAndAlias' => ['name' => 'alias_p'],
146+
'pathPropForAnotherController' => ['name' => 'pathPropForAnotherController'],
146147
];
147148

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

0 commit comments

Comments
 (0)