Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Deprecated form types as services#18343
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| <serviceid="form.type_guesser.validator"class="Symfony\Component\Form\Extension\Validator\ValidatorTypeGuesser"> | ||
| <tagname="form.type_guesser" /> | ||
| <argumenttype="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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why the" delimiters ?
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
This service should not be deprecated, as it has a dependency
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@HeahDude According tohttps://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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ok thanks for the heads-up :)
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.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@stof agreed on this one, but the others can safely be deprecated IMHO.
stof commentedMar 29, 2016
Status: needs work |
bb8844f to51b5b11CompareHeahDude commentedMar 29, 2016
Status: Needs Review |
10e816b to1bd0365Compare| * 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
form type extensions and form guessers are not deprecated (they cannot be)
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.
Yes I missed that.
HeahDude commentedMar 29, 2016
Status: Needs Work |
1bd0365 to7989b03CompareHeahDude commentedMar 29, 2016
| * 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this is still wrong for type extensions. Type extensions always need to be registered
| </service> | ||
| <serviceid="form.type_extension.repeated.validator"class="Symfony\Component\Form\Extension\Validator\Type\RepeatedTypeValidatorExtension"> | ||
| <tagname="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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@stof I don't think they have to since they don't have strict dependencies.
Right ?
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.
Please stop linking to form extensions to justify that these services are useless. As I already said many times, form extensions areNOT registered when using the fullstack framework, in favor of lazy-loading.
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.
Definitely understood. Sorry for the confusion about this, thanks for clarifying it... again :)
HeahDude commentedMar 29, 2016
Some tests are failing :/ Status: Needs Work |
HeahDude commentedMar 29, 2016
@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. |
stof commentedMar 29, 2016
@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) |
stof commentedMar 29, 2016
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 |
HeahDude commentedMar 29, 2016
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. |
HeahDude commentedMar 29, 2016
@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 ? |
HeahDude commentedMar 29, 2016
See#18356. |
HeahDude commentedMar 29, 2016
@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).