[FrameworkBundle] Deprecated form types as services #18343
Conversation
| @@ -43,6 +43,7 @@ | |||
| <service id="form.type_guesser.validator" class="Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser"> | |||
| <tag name="form.type_guesser" /> | |||
| <argument type="service" id="validator.mapping.class_metadata_factory" /> | |||
| <deprecated>"The \"%service_id%\" service is deprecated since Symfony 3.1 and will be removed in 4.0."</deprecated> | |||
There was a problem hiding this comment.
Properly copied from YAML :) @HeahDude you don't need to quote the message in XML or escape quotes.
There was a problem hiding this comment.
Yup, and I think it's gonna fail (it should, as it shouldn't detect the exact "%service_id%" if my memory serves me right)
There was a problem hiding this comment.
This service should not be deprecated, as it has a dependency
There was a problem hiding this comment.
I tried the SE without registering it and it worked. If the extension is lazy loaded so are the types.
There was a problem hiding this comment.
@HeahDude According to https://github.com/symfony/symfony/blob/master/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services9.xml#L90-L92, it should fail if you escape said " and put " delimiters though....
There was a problem hiding this comment.
Ok thanks for the heads-up :)
There was a problem hiding this comment.
@HeahDude if you remove this service, type guessing will not work properly anymore (it won't read validator metadata anymore).
Using the SE alone would not show it, as there is nothing in it relying on this feature (you have to write code using forms).
There was a problem hiding this comment.
@stof agreed on this one, but the others can safely be deprecated IMHO.
|
Status: needs work |
bb8844f to
51b5b11
Compare
|
Status: Needs Review |
10e816b to
1bd0365
Compare
| @@ -6,6 +6,7 @@ CHANGELOG | |||
|
|
|||
| * Added `Controller::json` to simplify creating JSON responses when using the Serializer component | |||
| * Deprecated absolute template paths support in the template name parser | |||
| * Deprecated services for core form types, form type extensions and form guessers | |||
There was a problem hiding this comment.
form type extensions and form guessers are not deprecated (they cannot be)
There was a problem hiding this comment.
Yes I missed that.
|
Status: Needs Work |
1bd0365 to
7989b03
Compare
| @@ -6,6 +6,7 @@ CHANGELOG | |||
|
|
|||
| * Added `Controller::json` to simplify creating JSON responses when using the Serializer component | |||
| * Deprecated absolute template paths support in the template name parser | |||
| * Deprecated services for core form types and core form type extensions without dependencies | |||
There was a problem hiding this comment.
this is still wrong for type extensions. Type extensions always need to be registered
| @@ -179,9 +208,11 @@ | |||
| </service> | |||
| <service id="form.type_extension.repeated.validator" class="Symfony\Component\Form\Extension\Validator\Type\RepeatedTypeValidatorExtension"> | |||
| <tag name="form.type_extension" extended-type="Symfony\Component\Form\Extension\Core\Type\RepeatedType" /> | |||
| <deprecated>The "%service_id%" service is deprecated since Symfony 3.1 and will be removed in 4.0.</deprecated> | |||
There was a problem hiding this comment.
You cannot deprecate these services. If an extension is not registered, it cannot extend the logic of the form type (and there is no way to automatically discover extensions based on the form type class name)
There was a problem hiding this comment.
@stof I don't think they have to since they don't have strict dependencies.
Right ?
There was a problem hiding this comment.
Please stop linking to form extensions to justify that these services are useless. As I already said many times, form extensions are NOT registered when using the fullstack framework, in favor of lazy-loading.
There was a problem hiding this comment.
Definitely understood. Sorry for the confusion about this, thanks for clarifying it... again :)
|
Some tests are failing :/ Status: Needs Work |
|
@stof Final call: I figured that actually all form types are lazy loaded using the only We cannot deprecate them at all. Sorry again for this confusion, I learned something important here! Closing though. |
|
@HeahDude actually, for any type not using dependencies, we can deprecate services, as form types are auto-registered on first usage in the new system (as the identifier is the class name, we can auto-register them when there is no dependency) |
|
however, marking services as deprecated while keeping them to preserve BC would indeed lead to warnings for any usage of the form types, as they would be registered using the services |
|
That's right, thank you so much for your feedback. I think we might be able to optimize things here, thanks to the refactoring of 2.8. I'm looking into it. |
|
@stof I've learned how it actually is very well optimised already and I find a way to get rid of those errors by just removing the tag. So we can deprecate them and keep them in case somebody has the weird idea to use them. What do you think ? |
|
See #18356. |
|
@stof thanks again for your patience and your lead on this, the config of the form registry is now crystal clear :) |
These services are not used anymore and should be removed in 4.0 (ref #15079).