Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[LiveComponent] Fix issue with onUpdated not triggered on nested array proprety #2013

Open
wants to merge 4 commits into
base: 2.x
Choose a base branch
from
+34 −6
Open

[LiveComponent] Fix issue with onUpdated not triggered on nested array proprety #2013

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0b0e7bb
Fix issue with onUpdated not triggered on nested array proprety
TheDutchScorpion Jul 30, 2024
b968810
Add onUpdated with array test to LiveComponentHydratorTest
TheDutchScorpion Jul 31, 2024
0ea98bc
Merge branch 'symfony:2.x' into feature/nested-property-on-updated
TheDutchScorpion Aug 1, 2024
6c46e0c
Add support for wildcard to onUpdated
TheDutchScorpion Aug 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/LiveComponent/src/LiveComponentHydrator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,8 @@ private function processOnUpdatedHook(object $component, string $frontendName, L

foreach ($onUpdated as $propName => $funcName) {
if (LiveProp::IDENTITY === $propName) {
if (!$dehydratedUpdatedProps->hasPropValue($frontendName)) {
if (!$dehydratedUpdatedProps->hasPropValue($frontendName)
&& !$dehydratedUpdatedProps->hasNestedPathsForProperty($frontendName)) {
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the right place... for a LiveProp IDENTITY the only thing that can change is... the prop itself no ? Is this not something we should do line 680+ ?

Copy link
Contributor Author

@TheDutchScorpion TheDutchScorpion Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think thats the best place either to check if there are changes to a array property. My intention was to tigger the onUpdated whenever there is a change within the array without providing a specific nested path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm wrong, but it's also strange that if the $propMetadata->onUpdated() is a string, the update hook only triggers if the property has been completely changed to another property and not if part of it has been modified.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that, but i fear doing so here will have side effects .. wdyt ?

}

Expand Down
11 changes: 11 additions & 0 deletions src/LiveComponent/src/Util/DehydratedProps.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ public function getNestedPathsForProperty(string $prop): array
return $nestedPaths;
}

public function hasNestedPathsForProperty(string $prop): bool
{
foreach ($this->propValues as $fullPath => $value) {
if (str_starts_with($fullPath, $prop.'.')) {
return true;
}
}

return false;
}

public function calculateUnexpectedNestedPathsForProperty(string $prop, array $expectedNestedPaths): array
{
$clone = clone $this;
Expand Down
19 changes: 19 additions & 0 deletions src/LiveComponent/tests/Integration/LiveComponentHydratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,25 @@ public function onEntireEntityUpdated($oldValue)
});
}];

yield 'onUpdated: with array' => [function () {
return HydrationTest::create(new class() {
#[LiveProp(writable: true, onUpdated: 'onNestedArrayUpdated')]
public array $array = [];

public function onNestedArrayUpdated($oldValue)
{
if ('Kevin' === $this->array['name']) {
$this->array['name'] = 'Simon';
}
}
})
->mountWith(['array' => ['name' => 'Ryan']])
->userUpdatesProps(['array.name' => 'Kevin'])
->assertObjectAfterHydration(function (object $object) {
self::assertSame('Simon', $object->array['name']);
});
}];

yield 'string: (de)hydrates correctly' => [function () {
return HydrationTest::create(new class() {
#[LiveProp()]
Expand Down