[LiveComponent] LiveProps not changed in url if current route has path parameters

[fracsi]

Since 2.28.0 the query string does not update with the new value if the current route contains any path parameters.

Reproducer: https://github.com/fracsi/live-component-url-error

1. On the homepage / - the page parameter is updated in the url when you change the param (?value=2, etc)
2. On a route that has a parameter (/dummy/{param}) the query string is not updated.

2.29.* does not solve the problem

[Kocal]

Hi,

I didn't check your reproducer yet (thanks for providing one!), but just to be sure since you said that 2.29.* didn't solve your issue, but did you try 2.29.2 version (which was released some hours before opening the issue)?

It contains #2988 that solve two issues on LiveProps in URL, and we have I think a test-case that covers your issue.

Can you confirm 2.29.2 didn't fix your issue?

Thanks!

[fracsi]

@Kocal
I tested with the latest version, hoped that it solves the problem. The reproducer uses the latest version as well.

[Kocal]

Thanks for the confirmation, I will investigate the issue next week

[fracsi]

@Kocal I tried to track down the issue.
I found that the UrlFactory::createPath method throws a Symfony\Component\Routing\Exception\MissingMandatoryParametersException, so null is returned in the createFromPreviousAndProps method (line 42)

In the reproducer:
Current uri is: /dummy/foo-bar
Current route: app_index_with_parameters
Current route params: ["param" => "foo-bar"]

The original route params are fully ignored when trying to generate the new url.

[smnandre]

I've come to the same conclusion, we probably need to adapt/refactor the UrlFactory.

I'm thinking this code has been changed to fix bugs, but would now deserve some "fresh start".

Noting here anyway some things we probably want to do:

• less methods, especially if called only once
• more early exit (see below)
• avoid hydrating the whole component when there is no Live props around
• early exit a bit more
• merge path parameters & query separately as they cannot collide
• did I say early exit ?

Maybe use a service subscriber ?

class LiveUrlSubscriber implements EventSubscriberInterface, ServiceSubscriberInterface
{
    use ServiceMethodsSubscriberTrait;

    private const URL_HEADER = 'X-Live-Url';

    public static function getSubscribedEvents(): array
    {
        return [
            KernelEvents::RESPONSE => 'onKernelResponse',
        ];
    }

    public function __construct(
        protected ContainerInterface $container,
    ) {
    }

   // ...

    #[SubscribedService]
    private function router(): RouterInterface
    {
        return $this->container->get(__METHOD__);
    }

    #[SubscribedService]
    private function metadataFactory(): LiveComponentMetadataFactory
    {
        return $this->container->get(__METHOD__);
    }

    #[SubscribedService]
    private function componentHydrator(): LiveComponentHydrator
    {
        return $this->container->get(__METHOD__);
    }

Early exits

 public function onKernelResponse(ResponseEvent $event): void
    {
        if (
            !$event->isMainRequest()
            || !$event->getResponse()->isSuccessful()
            || !$event->getRequest()->attributes->has('_live_component')
            || !$event->getRequest()->attributes->has('_mounted_component')
            || !$event->getRequest()->headers->has(self::URL_HEADER)
        ) {
            return;
        }

        $mountedComponent = $event->getRequest()->attributes->get('_mounted_component');

        // TODO iterate on mounted props
        // do not dehydrate the component before, as it may not be needed
        [$pathProps, $queryProps] = $mountedComponent; // getLiveProps($mountedComponent);

        if ([] === $pathProps && [] === $queryProps) {
            // No props to generate a URL from, so we can skip this
            return;
        }

Simplify logic

I am sure i'm missing something here, but globally would the algorithm be reduced to ... one method (+ one to return live props)

// _ PSEUDO _ CODE _ 

// Path Props can ONLY be used for path parameters, not query parameters
// So we need to find the route and path params from the previous URL
if ([] !== $pathProps) {
    $currentContext = $this->router()->getContext();
    $context = RequestContext::fromUri($previousUrl);
         // match ?
        // $newPath = $this->router()->generate($match['_route'], [...$match['_route_params'], ...$pathProps]);
    $this->router()->setContext($currentContext);
}

// Query Props can ONLY be used for query parameters, not path parameters
// So we merge the query props with the previous URL's query parameters
$queryString = '';
if ([] !== $queryProps) {

    $previousQuery = parse_url($previousUrl, PHP_URL_QUERY);
    parse_str($previousQuery, $previousQueryParams);

    $queryString = http_build_query([
        ...$previousQueryParams,
        ...$queryProps,
    ]);
}

$event->getResponse()->headers->set(self::URL_HEADER, $newPath + '?' . $queryString);
}

[Kocal]

With your solutions, could it fix #2987 aswell?

[Kocal]

Opened #3053, can you confirm it fixes your issue? Thanks!