-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
Fix reset()
to ensure a property initially set to null
can be reset
#6729
Conversation
null
property
@Simoneu01 thanks for the PR! I've fixed the issue and made sure that the FormObject's reset test also passes (as per PR #6375 thanks to @markjaquith). Hope this helps! |
null
propertyreset()
to ensure a property initially set to null
can be reset
|
||
// Check if the property contains a dot which means it is actually on a nested object like a FormObject | ||
if (str($property)->contains('.')) { | ||
$propertyName = $property->afterLast('.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshhanley shouldn't it be afterFirst
& beforeFirst
? When I unset a.b.c
, then it's $component->a
that should be unset, not $component->{'a.b'}
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@levu42 Hmm not sure I exactly follow the abc example, but if you have a form object that has a $foo property that should be unset, then first you need to get $component->form and then unset $foo on the form object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshhanley When a form has an array, let's say $component->postForm
has a user
property as an array with user.name
class PostForm {
public array $user = [
'name' => 'Foo',
'email' => null,
];
}
And then in the component you do $this->reset('postForm.user.email')
, then the split should be postForm
& user.email
. With your code of afterLast
& beforeLast
, it would be split into postForm.user
& email
, no?
I know it's a super narrow edge case, and not sure, if it's possible at all, but I thought about it reading the code and wanted to just make sure there's not a new bug introduced quietly with this fix that then takes ages to debug in the future :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@levu42 Yep that's how I intended it to be split, because I need to see if email is already unset or not on portForm.user. Hence I use after last as the end property I need is email. The data_get handles the postForm.user part. Does that makes sense?
Ps sorry about poor formatting, on mobile 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshhanley that makes sense! Thanks for explaining :)
Lovely, thanks! |
Failing test for #6540
When the
reset()
method is called on a property with defaultnull
will destroy the property from the component.Livewire\Exceptions\PropertyNotFoundException : Property [$nullProp] not found on component:
With Livewire V2 instead when the
reset()
method was called it would reset the property to null