[Form] Allow Form events to be used as expected with inherit_data option
#40515
Conversation
PRE_SET_DATA and POST_SET_DATA to be used with inherit_data optionPRE_SET_DATA and POST_SET_DATA to be used with inherit_data option
d6aff48 to
c833e0f
Compare
b0c7af7 to
46be0ae
Compare
PRE_SET_DATA and POST_SET_DATA to be used with inherit_data optioninherit_data option
adbecf2 to
2ae62b6
Compare
inherit_data optioninherit_data option
|
I am a bit worried that merging this would break existing applications that rely on the fact that their listeners are not called on forms that have the |
|
Should this behavior enabled with an option? |
|
Do you have a concrete use case in mind where removing this limitation would be useful? |
|
Using Let's suppose we have a form child we want to render as There is no way to achieve that without |
bbdd335 to
3d1f8d4
Compare
# Conflicts: # src/Symfony/Component/Form/Form.php
c0fdb09 to
e1ccfe3
Compare
|
Any chance of moving this forward? It seems extremely counter-intuitive that some FormEvents are called on child forms and others aren't. |
| if ($dispatcher->hasListeners(FormEvents::PRE_SET_DATA)) { | ||
| $event = new PreSetDataEvent($form, $modelData); | ||
| $dispatcher->dispatch($event, FormEvents::PRE_SET_DATA); | ||
| $modelData = $event->getData(); |
There was a problem hiding this comment.
This would not allow child forms with inherit_data to change the model data of the parent form. This is a very bad idea IMO (and was one of the reasons why this was not supported)
| } | ||
| $event = new PreSubmitEvent($form, $eventSubmittedData); | ||
| $dispatcher->dispatch($event, FormEvents::PRE_SUBMIT); | ||
| $submittedData = $event->getData(); |
There was a problem hiding this comment.
This is broken. It will replace the whole submitted data by the data of the child only.
Why do you consider them incompatible? If this is by design it should be documented that some events trigger every time and others don't. |
|
@Amunak see my comments on those lines that describe the problems. |
|
@stof I understand. But if this is incompatible by design, wouldn't be better to throw an exception when you use |
This PR allows
PRE_SET_DATA,POST_SET_DATAandSUBMITform events to be used also withinherit_dataoption set totrue.On top of that, usage of
PRE_SUBMITandPOST_SUBMITwas allowed withinherit_dataoption but dispatched events have$event->getData()asnull. This PR addresses that issue as well.Even if the documentation explains (at the very bottom of this page) that those form events are not going to work when the option
inherit_datais used, I think it's a common problem you could run into with an advanced use of Form component (also because no exception is thrown when you use*_SET_DATA,SUBMITandinherit_datatogether).This is important to keep the high consistency standard Form component has.
Without this PR, the form component behaviour is ambiguous but known and documented, so I think we should consider this a new feature instead of a bug fix.