-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Form] Add AsFormType attribute to create FormType directly on model classes
#60563
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
base: 8.1
Are you sure you want to change the base?
Conversation
|
It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you. Cheers! Carsonbot |
82ed4dc to
2cfe438
Compare
884ad5e to
2eb6ef0
Compare
|
I do like the feature, but thinking about an extreme example of going all-in on attributes, I wonder were it goes. Small simple example of building an app where we store informations about Events sent by users with our own schema, and we also receive informations from APIs with their own structure (you see where this is going). So we need the usual ORM attributes, some Validation, Object-Mapper, and these Form attributes. The entity becomes quite heavy on attributes. use App\Dto\Event as EventDTO;
use App\Repository\EventRepository;
use Doctrine\DBAL\Types\Types;
use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Form\Attribute\AsFormType;
use Symfony\Component\Form\Attribute\Type;
use Symfony\Component\Form\Extension\Core\Type as FormType;
use Symfony\Component\ObjectMapper\Attribute\Map;
use Symfony\Component\Validator\Constraints as Assert;
#[AsFormType]
#[Map(target: EventDTO::class)]
#[ORM\Entity(repositoryClass: EventRepository::class)]
class Event
{
#[ORM\Id]
#[ORM\GeneratedValue]
#[ORM\Column]
private ?int $id = null;
#[Map(target: 'eventName')]
#[Assert\Length(min: 15)]
#[Assert\NotBlank()]
#[ORM\Column(length: 255)]
#[Type]
private ?string $name = null;
#[Map(target: 'synopsis')]
#[Assert\NotBlank()]
#[ORM\Column(type: Types::TEXT)]
#[Type(FormType\TextareaType::class)]
private ?string $description = null;
// ...As you can see, the business need is small, there's not much in terms of validation or mapping, and I've included only few properties. I don't know if that's a problem honestly, it's not that unreadable to me, by I do think the question should be raised. That being said, once again, this is a really nice and neat feature, great job! |
I think in your usecase if you map object to a dto then the form should probably be on said dto. WDYT ? |
I know you love teasing me 😉 . Just to keep you happy:
I don't pretend this usecase is universale though. Once again, it's designed to create a fake extreme example, to show what it could lead. It's more of a "let's pretend there's a need like that". And then again, I don't say I'm definitely against this, I love the feature. But I also like to raise questions ;) |
|
This look like a fantastic addition for simple forms, but honestly I'm not 100% sure for mid-level and complex forms. PHP attributes are excellent for declaratively adding metadata, but there are limitations and cases where attributes are not the best fit. For instance, dynamic field definition, custom business logic, injecting services, or conditional configuration. In those cases, the traditional FormType classes may still be preferable (as they are services by definition). I don’t think we should replicate all form type capabilities with attributes (because the scope and dynamic limitations), but they can certainly be useful for building simple forms quickly, and then easily switch to FormType classes when things get more complex. |
|
@yceruto : Yes this is a totally valid point. I don't think attributes are meant to replace Forms. But should try to cover most common cases without the need to refactor it to a dedicated class. Might be worth adding a warning to the documentation of this feature the list of known limitations. |
$form = $formFactory->create(UserDTO::class, $user);It looks strange from the design PoV passing the DTO class as form type. I’m wondering if, instead of passing the data class, we can leave the form type param as $form = $formFactory->create(null, $user); // the form type will be guessed and defined from the DTOMaybe the same guesser mechanism could help with this? It'd be aligned with the null form type concept. |
|
@Tiriel Yes it can be used directly on entities, but even with your example, to make it more readable, a dedicated DTO for the form can still be done 😉 @yceruto I think its a good start to keep it simple yes. We just need to set the limit to which capabilities we want to replicate. I also thought of a command that can "convert" an attribute form to a traditional And Im totally agree to something like : $form = $formFactory->create(null, $user);
// or even?
$form = $formFactory->create($user); |
| * @author Benjamin Georgeault <git@wedgesama.fr> | ||
| */ | ||
| #[\Attribute(\Attribute::TARGET_PROPERTY)] | ||
| final readonly class Type |
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.
To stay consistent with the purpose of this attribute, as described in the class description, it represents a form field, so I’d prefer to name this class Field ?
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 think Field is wrong. Because it's biased by how it displays. But a "Field" could be an entire form. so I think we should keep Type but maybe rename other occurences of Field's to Type's. WDYT ?
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.
Maybe a description like this is more clear and accurate:
* Can be added to class property, in cases where the class is marked with {@see AsFormType} attribute.
* It defines a field for this form type (can by a nested group of fields too).
IMHO, the term field in the description is still ok. If you take a look at FormBuilderInterface::add description, the same word is used to describe it.
And agreed with @Neirda24 to keep it Type as 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.
@yceruto Any thought?
In that case, what would be the benefit of using a new dedicated DTO+ attribute over a regular FormType class? |
There are implicit limitations with options and Closures, which may be mitigated in PHP 8.5, but still, adding large blocks of logic in attributes feels a bit messy to me. I think advanced features like data mappers, form events, and model/view transformers carry special complexity and responsibility tied to the form type itself, but I’m open to making them configurable using attributes, as long as we can also implement them outside the DTO. |
Yes, but what about just creating a command that generate the form type from a DTO 😅? |
even |
src/Symfony/Component/Form/Tests/Extension/Metadata/Type/MetadataTypeTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Form/Tests/Extension/Metadata/Type/MetadataTypeTest.php
Outdated
Show resolved
Hide resolved
Symfony does not guarantee named arguments on methods at the moment. So maybe better not show this on documentation / example ? |
|
@Tiriel Still one class less, less code:
Yes, using the closure inside attributes in 8.5 can remove some limitations.
Agreed
Can still be done, those 2 features are not mutually exclusive 😄 |
I don't think we're talking about the same thing and we're going deep into things not strictly related to this feature. I don't see the point of the DTO in the first traditional case. And so, the number of classes is the same. But that's beside the point. I'm not against you or the feature, I'm just saying that there are use cases where this could lead to bloated entities. And I'm not sure that advising user to put the attributes on another unnecessary class is the best solution. But then again I seem to be the only one fearing that so maybe it's not that much of a concern. |
GromNaN
left a comment
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 am very enthusiastic about this feature after seeing your slides: https://speakerdeck.com/wedgesama/symfonyonline-june-2025-simplify-your-symfony-form-with-php8-attributes
I have noticed some improvements in integration with the framework.
There is a topic on reading attributes, which is better done at compilation rather than at runtime. However, this restrict the use of closures in attributes (for event listeners).
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
| $container->setParameter('form.type_extension.csrf.enabled', false); | ||
| } | ||
|
|
||
| if ($config['form']['use_attribute']) { |
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.
Using attributes is already opt-in as you have to add the attribute to a class that is autoconfigured. I understand the benefit of this new configuration in avoiding the declaration of unnecessary services, but it would be simpler for users of the framework if they did not need to activate this option.
Unnecessary service (form.metadata.attribute_loader) can be removed in the compiler pass.
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.
Maybe switch to enable by default instead of disable by default in src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php to keep the possibility to disable, WDYF?
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.
If no form.metadata.form_type tags exist, the services in the CompilerPass can be removed, eliminating the need for the use_attribute configuration.
src/Symfony/Component/Form/Extension/DataCollector/FormDataExtractor.php
Outdated
Show resolved
Hide resolved
2348039 to
484bc0a
Compare
626fd87 to
0ccc73f
Compare
| * @author Benjamin Georgeault <git@wedgesama.fr> | ||
| */ | ||
| #[\Attribute(\Attribute::TARGET_CLASS)] | ||
| class AsFormType |
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 don't like the name AsFormType as it looks like it will register the service as a form type (tagging it as form.type), while it has a totally different behavior (it defines that class as providing metadata to configure a form type, not as being a form type)
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.
What do you think of:
AsFormMetadataFormMetadataAsForm
I have a preference for the 1. And if you have ideas 🙏
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.
IMO even if it is not a Form type per se, it is used as a form type. same goes for AsController. It marks the class a controller when in fact it is the methods
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 WDYT of answers?
ef9388c to
59f0ab8
Compare
|
Nice, but I remain skeptical about replicating all the form features (currently your WIP Form Event & Data Transformer) directly in the class object. Even if this new approach is optional, there’s a significant risk it could lead to messy code for some users, IMO. |
136c4ee to
a79a967
Compare
a79a967 to
c3c026f
Compare
For those interested in this feature, I’ve released my alternative implementation with the v1 of AutoFormBundle (and optionally v4 of TranslationFormBundle to automatically handle Gedmo/Knp translation cases). |
Form Attribute
This PR is about how to use PHP Attribute to build a Symfony Form.
Build your form directly on your data (e.g. on a DTO but not limited to it 😉).
Brainstormed with @Jean-Beru @Neirda24 and @smnandre
EDITED: !!! Disclamer !!!
This PR will not replace nor deprecate anything in the Form Component. This is just syntactic sugar 😉
Description
What in mind when starting this:
FormTypeclassdata_classmandatory but implicit)Data first, Form secondinstead ofForm first, Data second" quoted from @Neirda24 😄FormType(easy because in the background, it is still aFormType😉)Here what it looks like with a basic example:
Basic features (implemented in this PR)
Basic form
| - With Attributes
| - Classic equivalence (without attribute)
| - Variant 1, with one attribute per FormType (Not implemented)
Thought of that too, I like the readability but maybe too much work to maintain.
Still need a 'GenericType' to work with type that does not have attribute equivalence.
With validation
Work like a charm, without to do anything more to support it
Class inheritance
| - With attribute
| - Classic equivalence (without attribute)
Next features (WIP)
Here some DX example of how to implement next features from form component with attributes.
For Form Event and Data Transformer, can be more powerful with PHP 8.5 closures_in_const_expr in the future \o/
Form Event
| - Event on the root FormType
| - With Attributes
| - Classic equivalence (without attribute)
| - Event on a FormType's field
| - With Attributes
| - Classic equivalence (without attribute)
Data Transformer
| - DT on the root FormType
| - With attribute
| - Classic equivalence (without attribute)
| - DT on a FormType's field
| - With attribute
| - Classic equivalence (without attribute)
| - Variant 1 with DataTransformerInterface
Not a fan but.
| - Variant 2, with 2 attributes
Use 2 distinct attribute, one for model transformer, another for view transformer.
Form Type Extension
Auto Form \o/
Need to be discussed / Possible evolution
Here stuff that I am no satisfy for now and need to be discuss to find a better way to do it.
Autowire
| - In options
| - Event
| - Data Transformer
configureOption method
| - Varaint 1 (not fan of it)
| - Varaint 2 (love it but PHP 8.5)
Override getParent
Not sure if this means something, maybe not needed. What do you think?
Symfony Demo Applicationwith Form AttributeYou can try the basic examples of
AsFormTypein theSymfony Demo Applicationfork:https://github.com/WedgeSama/demo-with-form-attribute/tree/demo-with-form-attribute
List of
FormTypereplaced (examples on both DTO and entities):Form\ChangePasswordType=> created DTODTO\UserChangePasswordDTOForm\UserType=> created DTODTO\UserProfileDTOForm\CommentType=> on existing entityEntity\CommentForm\PostType=> on existing entityEntity\PostIt use git submodule to require the form component with form attribute.
PS: any suggestion are welcome 😉