[Form] trigger deprecation warning when using empty_value #15945
Conversation
|
Other implementations of renamed options default the original option to |
|
@wouterj Do you have an example? |
|
@xabbuh for instance the |
|
@wouterj The thing is that the default value of the |
|
The virtual example is non-optimal as well. Best appraoch is (which I used for the options i deprecated).
Otherwise deprecations would not be triggered if the new option is overwritten but the old option is still used (which would then break in 3.0 as the option doesn't exist anymore). |
|
Hmm, the actual problem here is that |
|
Just use a different value as default then that doesn't have a real meaning,e .g. |
| @@ -179,7 +179,11 @@ public function configureOptions(OptionsResolver $resolver) | |||
| return $options['required'] ? null : ''; | |||
| }; | |||
|
|
|||
| $placeholder = function (Options $options) { | |||
| $placeholder = function (Options $options) use ($emptyValue) { | |||
| if ($emptyValue !== $options['empty_value']) { | |||
There was a problem hiding this comment.
This cannot work anyway from what I can see. $emptyValue is a closure.
|
Status: Needs Work |
The `empty_value` option is deprecated in the `choice`, `date`, and `time` form types. Therefore, a deprecation warning must be triggered when the users configures a value for this option. The `datetime` form type does not need to be updated as it passes configured values to the `date` and `time` form types which trigger deprecation warnings anyway.
|
I updated here. Status: Needs Review |
| @@ -328,7 +329,7 @@ public function configureOptions(OptionsResolver $resolver) | |||
| 'preferred_choices' => array(), | |||
| 'group_by' => null, | |||
| 'empty_data' => $emptyData, | |||
| 'empty_value' => $emptyValue, // deprecated | |||
| 'empty_value' => new \Exception(), // deprecated | |||
There was a problem hiding this comment.
This is not really nice, but as the empty value can be nearly everthing it was best I could think of.
There was a problem hiding this comment.
It's quite unfortunate choice, as exceptions include stacktrace, which can cause circular references if used in functional tests. My Behat test suite with new \Exception() uses 200MB of memory, but with null it only uses 103MB. Can we use some other object instead? :)
|
Thank you @xabbuh. |
…(xabbuh) This PR was merged into the 2.7 branch. Discussion ---------- [Form] trigger deprecation warning when using empty_value | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #15908 | License | MIT | Doc PR | The `empty_value` option is deprecated in the `choice`, `date`, and `time` form types. Therefore, a deprecation warning must be triggered when the users configures a value for this option. The `datetime` form type does not need to be updated as it passes configured values to the `date` and `time` form types which trigger deprecation warnings anyway. Commits ------- 405d4a8 trigger deprecation warning when using empty_value
This PR was merged into the 2.7 branch. Discussion ---------- [Form] fixed "empty_value" option deprecation | Q | A | ------------- | --- | Branch? | 2.x only | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #15945 (comment) | License | MIT | Doc PR | ~ I didn't make any profiling but a resolver instance is passed to `configureOptions()` creating locale variables including those exceptions for each field using one of the patched form types, so I guess the memory usage can grow really fast. Commits ------- 7e84907 [Form] fixed "empty_value" option deprecation
This PR was merged into the 2.7 branch. Discussion ---------- [Form] fixed "empty_value" option deprecation | Q | A | ------------- | --- | Branch? | 2.x only | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony/symfony#15945 (comment) | License | MIT | Doc PR | ~ I didn't make any profiling but a resolver instance is passed to `configureOptions()` creating locale variables including those exceptions for each field using one of the patched form types, so I guess the memory usage can grow really fast. Commits ------- 7e84907 [Form] fixed "empty_value" option deprecation
The
empty_valueoption is deprecated in thechoice,date, andtimeform types. Therefore, a deprecation warning must be triggeredwhen the users configures a value for this option.
The
datetimeform type does not need to be updated as it passesconfigured values to the
dateandtimeform types which triggerdeprecation warnings anyway.