[LiveComponent] Alias URL bound props #1396

squrious · 2024-01-16T12:14:01Z

Q	A
Bug fix?	no
New feature?	yes
Issues	N/A
License	MIT

Following #1230.

Allow custom parameter names for URL bound props, and mapping specification from Twig templates.

Usage

From PHP definition:

#[AsLiveComponent()]
final class MyComponent
{
    // ...

    #[LiveProp(writable: true, url: new QueryMapping(alias: 'q'))
    public ?string $search = null;
}

From templates:

{{ component('MyComponent', {
     'data-live-url-mapping-search': {
       'alias': 'q'
      }
   }) 
}}
{{ component('MyComponent', { 'data-live-url-mapping-search-alias': 'q' }) }}

HTML syntax also works:

<twig:MyComponent :data-live-url-mapping-search="{ alias: 'q'}" />
<twig:MyComponent data-live-url-mapping-search-alias="q" />

Result

Changing the value of search will update the url to https://my.domain?q=my+search+string.

Mappings provided in Twig templates are merged into those provided in PHP. Thus, query mappings in PHP act as defaults, and we can override them in templates (e.g. for specific page requirements). So a page with:

<twig:MyComponent/>
<twig:MyComponent data-live-url-mapping-search-alias="q" />

will update its URL to http://my.domain?search=foo&q=bar.

smnandre · 2024-01-16T17:39:40Z

Is it accessible in PHP easily ?

squrious · 2024-01-17T11:39:26Z

Is it accessible in PHP easily ?

It's in the LiveProp metadata, so yes

WebMamba · 2024-01-17T16:52:06Z

src/LiveComponent/src/Attribute/LiveProp.php

         */
-        private bool $url = false,
+        private bool|QueryMapping $url = false,


Instead of having the mapping in the url argument, is it not more readable to have a separate argument alias?

I think I like it inside of url as a QueryMapping. There will be other options coming in the future too. But we could talk about the naming: maybe UrlMapping to match the url config?

weaverryan · 2024-01-17T17:26:19Z

Thanks for this!

What's the use-case for needing to do:

data-live-url-mapping-search-alias="q"

This doesn't look like something that Livewire has (I check there as a quick way to validate the value of features).

Also: QueryMapping(alias: 'q') or QueryMapping(as: 'q')? as stolen from Livewire :p

smnandre · 2024-01-17T19:43:36Z

Is it accessible in PHP easily ?

It's in the LiveProp metadata, so yes

But... it's on a call per call basis... so a call to metadataFactory would not bring this data right ? So it's not usable in events, cache, etc ?

(just want to clarify when/where this data will be exposed, accessible, etc)

smnandre · 2024-01-18T07:55:08Z

I'm not sure to understand how the live prop mapping is "kept" during live action / event.. but it's probably just me needing coffee..

squrious · 2024-01-18T08:38:02Z

@weaverryan

What's the use-case for needing to do:

data-live-url-mapping-search-alias="q"

If the users just want to override this mapping property they can do it with this shortened syntax instead of passing an object. It may also be useful in the future when the mapping object contain other options: it will be easier to conditionally render specific attributes in this way, than setting up an object, IMO.

This doesn't look like something that Livewire has (I check there as a quick way to validate the value of features).

That's true, but I tried the Livewire feature, and honestly I missed it. As far I know, you can't avoid parameter name collision in Livewire, but I find it pretty useful.

Also: QueryMapping(alias: 'q') or QueryMapping(as: 'q')? as stolen from Livewire :p

I'm totally open for any renaming :) as looks good to me!

@smandre

But... it's on a call per call basis... so a call to metadataFactory would not bring this data right ? So it's not usable in events, cache, etc ?

The metadata defined in your PHP component is available through the metadata factory. If you override the mapping in a template, the overridden values will not be available from metadata. Though there is a utility class that helps in getting those values from mounted attributes.

I'm not sure to understand how the live prop mapping is "kept" during live action / event.. but it's probably just me needing coffee..

The mapping is render as a Stimulus value here.

smnandre · 2024-01-18T22:14:20Z

I'm still not sure if i'm paranoiac ... but if you're telling me that the "mapping configuration" is read and used from request data ...

I'm 90% sure i still miss something so i stop here :)

squrious · 2024-01-24T11:54:26Z

I renamed QueryMapping to UrlMapping, alias to as, and rebased to add some doc on this feature :)

weaverryan · 2024-01-29T15:25:16Z

src/LiveComponent/doc/index.rst

+    <twig:SearchModule :data-live-url-mapping-query="{ alias: 'q'}" />
+
+    {# Or configure the mapping option directly #}
+    <twig:SearchModule data-live-url-mapping-query-alias="q" />


Let's talk about this approach :). What I don't love about it is that users need to know about more data attribute options. And we need to make sure we keep this value on the frontend so we can reference it on re-renders.

The Livewire solution is to allow the mapping to be done in a method if you have something custom: https://livewire.laravel.com/docs/url#using-the-querystring-method

We could even go more generic: adding some way of configure a LiveProp entirely:

#[AsLiveComponent] class SearchModule { #[LiveProp(writable: true, 'modifier' => 'modifyQueryProp')] public string $query = ''; #[LiveProp] public string $alias = null; public function modifyQueryProp(LiveProp $prop): void { if (!$this->alias) { return; } $prop->url = new UrlMapping(as: $this->alias); } }

Usage:

<twig:SearchModule alias="q" />

A tiny bit more work for the user, but I think it's more understandable (and will also be more flexible). WDYT?

Oh I like it! It looks even easier to set up. I have a few questions in mind:

Does the alias prop need to be a LiveProp?

We need to make LiveProp writable in some way, is there any drawback?

The modifier method will need the component to be mounted before it's called, so we can use data passed from templates. Are we sure all LiveProp options are safe to be overridden here?

Awesome!

Does the alias prop need to be a LiveProp?

I think so, yes - because we need some way to persist this setting across re-renders. You were doing that before by making sure to always render the attribute. This, at least, uses a known mechanism.

We need to make LiveProp writable in some way, is there any drawback?

There could be, yes - you're correct to worry about this. Currently, you can grab a LiveProp[] from a component in a static way: that metadata is always the same for a given component class. I've just started looking at the component. The LiveComponentMetadata (which holds the LiveProp info) is nicely created only by the LiveComponentMetadataFactory. This would be the ideal situation: if we could continue to rely on LiveComponentMetadataFactory::getMetadata() everywhere and it would handle this new modifier config).

However, the signature currently is:

public function getMetadata(string $name): LiveComponentMetadata

It would need to become:

public function getMetadata(string $name, object $component): LiveComponentMetadata

Looking at the code that calls this, that's a problem in at least one spot: ChildComponentPartialRenderer. When a parent component re-renders, this component is responsible for figuring out whether or not the child components need to re-render (based on if the input props changed to the child). To do this, it needs the LiveComponentMetadata so it can read an updateFromParent option on each LiveProp. If we do not need to re-render the child, then we even avoid creating it. See the problem? With my idea, we would need to mount/create the child component in order to see if it needs to be re-rendered.

But, this is the ONLY place where that's a problem. A git grep '\->getMetadata(\$' src/LiveComponent shows a few other places where we call this method, but all of the have access to the $component object. In other words adding a 2nd argument would not be a problem. The LiveProp would need to become mutable... though if we want, we could actually keep it immutable and use ->with() methods to modify it - e.g.

- $prop->url = new UrlMapping(as: $this->alias); - return $prop->withUrlMapping(new UrlMapping(as: $this->alias));

Where ->withUrlMapping() creates a new instance of itself. A bit more work for the user, but this is not a main use-case.

If we like this, we could work around the ChildComponentPartialRenderer, I think. This could be ONE spot where do something different to find these LiveProp with updateFromParent (and this property would NOT have a with() method to change it dynamically). We could, for example, add an internal LiveComponentMetadataFactory::getMetadataWithoutModifiers(string $name) that would do what it does now: return the metadata but without calling the modifier). It would be internal and only used in the one special case of ChildComponentPartialRenderer.

The modifier method will need the component to be mounted before it's called, so we can use data passed from templates. Are we sure all LiveProp options are safe to be overridden here?

Based on the above research and because all metadata is created from LiveComponentMetadataFactory, I think yes, they are all safe to be changed except for updateFromParent.

Thanks for the investigations! So now this "side feature" of the alias option is growing up. I would remove it from this PR and work on it in a dedicated one, where we could specifically address the ChildComponentPartialRenderer modifications you mentioned above. Wdyt?

Ha! Indeed it is - it's exposed a bigger thing!

I would remove it from this PR and work on it in a dedicated one, where we could specifically address the ChildComponentPartialRenderer modifications you mentioned above. Wdyt?

It'd be great to focus on that part in a smaller PR. But as LiveComponents will become stable at the end of Feb, I don't want to merge this now, if it'll change later.

If you'd like, there could be a compromise: perhaps you create a PR now with this refactoring of the getMetadata() method and ChildComponentPartialRenderer? Then we get that merged and then update and merge this PR.

Thanks for your patient work - you PR's have proved to be complex in a good way: handling important things in the correct way.

Alright, let's do this!

Thanks for your patient work - you PR's have proved to be complex in a good way: handling important things in the correct way.

Thank you!

This PR was squashed before being merged into the 2.x branch. Discussion ---------- [LiveComponent] Add modifier option to LiveProp | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | Issues | N/A | License | MIT Hi! Following discussions in [#1396](#1396 (comment)), here is a proposal to allow a `LiveProp` to be modified at runtime. The feature adds a `modifier` option to `LiveProp`, so users can use a custom method to modify the `LiveProp`'s options depending on the context. This context could be a service, or even another prop within the component. ### Usage Here is the example given in the doc: ```php #[AsLiveComponent] class ProductSearch { #[LiveProp(writable: true, modifier: 'modifyAddedDate')] public ?\DateTimeImmutable $addedDate; #[LiveProp] public string $dateFormat = 'Y-m-d'; // ... public function modifyAddedDate(LiveProp $prop): LiveProp { return $prop->withFormat($this->dateFormat); } } ``` ```twig {{ component('ProductSearch', { dateFormat: 'd/m/Y' }) }} ``` ### Changes overview First, we have to make the `LiveProp` attribute editable in some way. I opted out for immutable methods, so caching is preserved when applicable. Then add a `LivePropMetadata::withModifier(object $component)` method that returns a new instance modified by the user-defined method. This is called in a few places, similarly to `LiveComponentMetadata::calculateFieldName()`. The tricky part is component hydration. If we use props in modifiers to change other `LiveProp` options, they have to be hydrated before the modifier is applied. This is only useful for options related to hydration, but it concerns most of them... The trick is to sort metadata, so modified ones are hydrated at the end. Commits ------- 3bcc5c2 [LiveComponent] Add modifier option to LiveProp

squrious · 2024-03-06T11:26:33Z

Hi there!

Just rebased to get the LiveProp modifier feature.

I also rewrote the test component for url bindings. We often add use cases and I found it becomes hard to understand things like "prop1", "prop2", etc in assertions. This is in a separate commit, so if it's not the best place do do this it can be removed easily ;)

Doc section is also updated!

smnandre · 2024-04-08T23:23:52Z

@squrious where are we here ? Do you need something / is it "ready" for you ?

squrious · 2024-04-09T07:14:31Z

@squrious where are we here ? Do you need something / is it "ready" for you ?

Hi @smnandre! It's ready for me

src/LiveComponent/src/Metadata/UrlMapping.php

kbond

This is great @squrious! One small nitpick.

kbond · 2024-04-16T14:30:02Z

Thanks Nicolas for your time and patience on this!

WebMamba reviewed Jan 17, 2024

View reviewed changes

squrious force-pushed the live-component/url-alias branch from 5a0976a to 2ba3c69 Compare January 24, 2024 11:52

weaverryan reviewed Jan 29, 2024

View reviewed changes

weaverryan added Status: Waiting Feedback Needs feedback from the author Feature New Feature labels Jan 29, 2024

squrious mentioned this pull request Feb 16, 2024

[LiveComponent] Add modifier option to LiveProp #1507

Merged

squrious force-pushed the live-component/url-alias branch 2 times, most recently from 9063962 to 21af89f Compare March 6, 2024 11:19

squrious force-pushed the live-component/url-alias branch from 21af89f to c284a63 Compare March 18, 2024 09:51

kbond reviewed Apr 16, 2024

View reviewed changes

src/LiveComponent/src/Metadata/UrlMapping.php Outdated Show resolved Hide resolved

kbond requested changes Apr 16, 2024

View reviewed changes

carsonbot added Status: Needs Work Additional work is needed Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels Apr 16, 2024

[LiveComponent] Alias URL bound props

828e34e

kbond force-pushed the live-component/url-alias branch from ea57558 to 828e34e Compare April 16, 2024 14:29

kbond merged commit d4df614 into symfony:2.x Apr 16, 2024
35 of 36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LiveComponent] Alias URL bound props #1396

[LiveComponent] Alias URL bound props #1396

squrious commented Jan 16, 2024

smnandre commented Jan 16, 2024

squrious commented Jan 17, 2024

WebMamba Jan 17, 2024

weaverryan Jan 17, 2024 •

edited

Loading

weaverryan commented Jan 17, 2024 •

edited

Loading

smnandre commented Jan 17, 2024

smnandre commented Jan 18, 2024

squrious commented Jan 18, 2024

smnandre commented Jan 18, 2024

squrious commented Jan 24, 2024

weaverryan Jan 29, 2024 •

edited

Loading

squrious Jan 30, 2024

weaverryan Jan 30, 2024

squrious Jan 30, 2024

weaverryan Feb 7, 2024

squrious Feb 9, 2024

squrious commented Mar 6, 2024

smnandre commented Apr 8, 2024

squrious commented Apr 9, 2024 •

edited

Loading

kbond left a comment

kbond commented Apr 16, 2024

[LiveComponent] Alias URL bound props #1396

[LiveComponent] Alias URL bound props #1396

Conversation

squrious commented Jan 16, 2024

Usage

Result

smnandre commented Jan 16, 2024

squrious commented Jan 17, 2024

WebMamba Jan 17, 2024

Choose a reason for hiding this comment

weaverryan Jan 17, 2024 • edited Loading

Choose a reason for hiding this comment

weaverryan commented Jan 17, 2024 • edited Loading

smnandre commented Jan 17, 2024

smnandre commented Jan 18, 2024

squrious commented Jan 18, 2024

smnandre commented Jan 18, 2024

squrious commented Jan 24, 2024

weaverryan Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

squrious Jan 30, 2024

Choose a reason for hiding this comment

weaverryan Jan 30, 2024

Choose a reason for hiding this comment

squrious Jan 30, 2024

Choose a reason for hiding this comment

weaverryan Feb 7, 2024

Choose a reason for hiding this comment

squrious Feb 9, 2024

Choose a reason for hiding this comment

squrious commented Mar 6, 2024

smnandre commented Apr 8, 2024

squrious commented Apr 9, 2024 • edited Loading

kbond left a comment

Choose a reason for hiding this comment

kbond commented Apr 16, 2024

weaverryan Jan 17, 2024 •

edited

Loading

weaverryan commented Jan 17, 2024 •

edited

Loading

weaverryan Jan 29, 2024 •

edited

Loading

squrious commented Apr 9, 2024 •

edited

Loading