-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Don't require an object property to be readable for writing #36754
Comments
From my POV a form needs to display the current data and needs to accept user input to change this data. The solution proposed here suggest to skip the reading part (i.e. ignoring the "map data to forms" part of the data mapping layer). In my opinion this is not something that needs to be implemented in the core, but can be implemented in userland with a custom data mapper if someone needs it). So 👎 from me here. |
That is not the case. The reading would be only skipped where its value is not used. The reading there is just an implementation detail for keeping object references. The proposed solution suggests to treat reading and writing the same. |
I may be misunderstanding what this is about then, but your second example clearly fails because of the missing reading part, right? |
Yes, it does. Because there is a reading in place when trying to write. But it is just an implementation detail and the value is not used. class Foo2 {
private $bar;
public function setBaz($value) {
$this->bar = $value * 2;
}
public function getBaz($value):void {}
} Also reading class Foo2 {
private $bar;
public function getBaz() {
return $this->bar / 2;
}
} |
Yes, but both examples do not make much sense when being used with the Form component as either mapping the data to the form or the other way around won't work. |
I'm also not sure why one would want to map object data to a form if it can't be mapped back. Maybe sending it to a different endpoint / pre-filled form etc... But this one works. For mapping form data to an object that can't be mapped back: Think of cast-away-forms that are just filled by the user. These don't work. Regardless of the use case: Why make it more difficult for the developer to understand why his implementation fails than it needs to be?
|
I see it the other way around: Silencing some of the errors makes debugging much harder when you are wondering why mapping data to forms and vice versa is not working as expected. |
It would still throw an exception if you try to read. But the exception for the non-working read operation would point to a read operation and not to a write operation. |
Thank you for this suggestion. |
Friendly reminder that this issue exists. If I don't hear anything I'll close this. |
@carsonbot Yes, this still poses an undocumented limitation that results in confusing error messages. |
Maybe I'm mistaken but wouldn't this issue be resolved by #37520 ? |
@MatTheCat That merged PR only includes a part of the initial PR. I still think the exception is confusing and poses an undocumented limitation due to the implementation detail that we read when a write is performed. |
Oh I understand now 😅 so indeed it was not addressed. |
Thank you for this suggestion. |
Could I get an answer? If I do not hear anything I will assume this issue is resolved or abandoned. Please get back to me <3 |
Hey, I didn't hear anything so I'm going to close it. Feel free to comment if this is still relevant, I can always reopen! |
Description
As dicussed in #36492 the
Symfony\Component\Form\Extension\Core\PropertyPathMapper
does not behave as expected.When trying to write form values to an object it fails if the property is not readable.
The current implementation tries to get the value to avoid setting an identical value.
PropertyPathMapper
should catch any exceptions that are specific to reading the property when trying to write it.It might set an identical value.
If writing fails, an exception will still be thrown as expected.
Example
One should be able to map a form with
bar=123
toFoo1
, but it will fail withUninitializedPropertyException
.One should be able to map a form with
baz=123
toFoo2
, but it will fail with aNoSuchPropertyException
.The text was updated successfully, but these errors were encountered: