[LiveComponent] Fix new URL calculation with LiveProp using Serializer (and Serializer attributes), and when using custom modifier - #2988
Conversation
carsonbot
added
Bug
There was a problem hiding this comment.
I don't think this test was adding value, legit everything was mocked and it could not reflect the real behavior.
Instead I went for functional test
| $metadata = $this->metadataFactory->getMetadata($mounted->getName()); | ||
|
|
||
| $dehydratedProps = $this->liveComponentHydrator->dehydrate( | ||
| $mounted->getComponent(), | ||
| $mounted->getAttributes(), | ||
| $metadata | ||
| ); | ||
|
|
||
| $values = $dehydratedProps->getProps(); |
There was a problem hiding this comment.
This is the fix for the first bug
f6361c1 to
41e73d0
Compare
| }); | ||
| }]; | ||
|
|
||
| yield 'Array with DTOs: using serializer, fully writable allows partial changes' => [function () { |
There was a problem hiding this comment.
Since I added #[SerializedName(serializedName: 'c')] on Address::$city, our tests failed because we expected props to be dehydrated to city instead of c.
I duplicated these failing tests and changed the dehydrated property to c, and modified the previous tests to not use useSerializerForHydration.
|
cc @mbuliard, can you take a look? Thanks! |
| $address->city = 'Anytown'; | ||
| yield 'with an object in query, with "useSerializerForHydration: true", keys "address" and "c" must be present' => [ | ||
| 'previousLocation' => '/', | ||
| 'expectedLocation' => '/?intProp=3&objectPropWithSerializerForHydration%5Baddress%5D=123+Main+St&objectPropWithSerializerForHydration%5Bc%5D=Anytown', |
There was a problem hiding this comment.
That's not easy to read, but the c is present at 5Bc%5D
There was a problem hiding this comment.
Btw it would be nice if we could have a "clean" query, like it was done before.
I tried to play with http_build_query()'s encoding, but it didn't help
|
I pushed a second commit to fix #2991, now |
Kocal
changed the title
#[LiveProp] with useSerializerForHydration: true and #[SerializedName]#[LiveProp] and Serializer together, but also with custom modifier
Kocal
changed the title
#[LiveProp] and Serializer together, but also with custom modifierLiveProp using Serializer (and Serializer attributes), and when using custom modifier
| { | ||
| $urlMappings = []; | ||
| foreach ($this->livePropsMetadata as $livePropMetadata) { | ||
| foreach ($this->getAllLivePropsMetadata($component) as $livePropMetadata) { |
There was a problem hiding this comment.
This is the fix for the 2nd bug
2be1f38 to
a1b199e
Compare
…h `useSerializerForHydration: true` and `#[SerializedName]`
…h custom `modifier` which returns a new `LiveProp`
a1b199e to
19fe704
Compare
|
@Kocal Thanks for the fix 💙 Would you mind tagging it so we can test it out? |
|
I asked Kevin for a release but I'm not sure when it will happen, but in the meantime I think you can use 2.x-dev constraint version 🙂 |
|
@ruudk released in 2.29.2 |
3 participants
This PR fixes two bugs introduced #2673:
#[SerializedName(serializedName: 'p')]after 2.28 upgrade #2971 (comment) for explanationstl;dr: previously, query parameters were updated through
LiveComponentHydrator::dehydrate()which use the Serializer (ifuseSerializerForHydration: true) and so also used Serializer attributes likeSerializedName.LiveProp(url: new UrlMapping(as: 'foo'))with a custom modifier that returns a newLivePropwith a newUrlMapping, ex:then the prop was persisted as
fooand notalias_fooin the URL.