[Form] Replace passing type instances by class names #5321

webmozart · 2012-08-22T14:31:42Z

This ticket is a follow-up to #5221.

Currently, it is a common practice to use form types by instantiating them directly instead of registering them in the DIC:

<?php

// (1) creating forms
$form = $this->createForm(new MyFormType());

// (2) adding fields
$builder->add('field', new MyFieldType());

// (3) declaring parent types
public function getParent()
{
    return new MyParentType();
}

This practice has a series of advantages:

Easy and rapid coding, no extra configuration needed.
Support for IDE auto-completion.

But it also has limitations:

If (1), (2) or (3) occurs multiple times during the same request (e.g. within a collection form), caching is impossible and degrades performance. Removing this performance impact means adding the affected types to the DIC and rewriting all code that uses them.
(3) cannot inject dependencies into the parent type.
(2) does not make use of type extensions.

Because of these inconsistencies, I suggest to deprecate this API and replace it by the following:

<?php

// (1) creating forms
$form = $this->createForm('Vendor\Bundle\Form\MyFormType');
$form = $this->createForm('VendorBundle:MyFormType');
$form = $this->createForm(MyFormType::class); // PHP 5.5

// (2) adding fields
$builder->add('field', 'Vendor\Bundle\Form\MyFieldType');
$builder->add('field', 'VendorBundle:MyFieldType');
$builder->add('field', MyFieldType::class); // PHP 5.5

// (3) declaring parent types
public function getParent()
{
    return 'Vendor\Bundle\Form\MyParentType';
}
public function getParent()
{
    return 'VendorBundle:MyParentType';
}
public function getParent()
{
    return MyParentType::class; // PHP 5.5
}

The advantages of this approach are:

Support for class names and registered types is identical. No difference in performance, support for type extensions etc.
Easy and rapid coding, no extra configuration needed.

The downsides:

No IDE support until PHP 5.5 [1] (except if IDEs add special treatment for form types).
Types that receive arguments in their constructors must be registered in the DIC.

[1] assuming that https://wiki.php.net/rfc/class_name_scalars gets accepted

henrikbjorn · 2012-08-22T14:42:18Z

The downside Types that receive arguments in their constructors must be registered in the DIC is a major one. I have types that are useraware and gets the in the constructor. This is still a simple type and would suck to have to have them in the DIC. That can ofc be used in the same way by adding an option (but feels wrong).

willdurand · 2012-08-22T14:46:10Z

I don't like this downside either.

webmozart · 2012-10-07T06:29:37Z

@henrikbjorn Why your aversion against the DIC? Adding a type there is a two-liner... (three if you use XML). Additionally, you reduce complexity any time you use the type, since you don't have to take care of dependency handling anymore. If your type has dynamic dependencies that are not available in the DIC (such as the user), you can in fact use options.

Tobion · 2012-11-27T15:36:49Z

Btw, having FormType defined as service (but not tagged as form type) and injecting it into the controller to build a form from it, won't be supported anymore, too. Right? This approach does not suffer from any limitation you mention, e.g. no performance penalty since it's indeed a service. But it's wouldn't work anymore.
This is just a hint. I'm not saying, I'm against this change.

Edit: I guess, using a service that is not tagged as form type, is still slower because the resolving to a ResolvedFormType could potentially be done multiple times?

beberlei · 2012-11-27T15:42:13Z

-1

deprecating this API ok, but removing it completly would be a a major punch in the face, as it will probably be quite some work to adjust applications.

The problem is not having 1 form in the service container, it is a combination of context change (second file) and having N forms in my app. My apps normally have 1-2 times the number of forms that i have entities.

stof · 2012-11-27T16:39:11Z

@Tobion it does suffer from the same penalties as it is still re-adding the instance in the FormFactory each time.

webmozart · 2012-12-13T15:23:12Z

@Tobion as @stof said, the Form component needs to process FormType instances and convert them into ResolvedFormType instances. Where the FormType comes from does not matter.

webmozart · 2012-12-13T15:24:12Z

@beberlei I don't quite understand your -1? -1 on removing or -1 on deprecating?

Of course, we would first deprecate the API in order to remove it later.

webmozart · 2012-12-13T15:26:38Z

Another thing we can do to ease the resulting pain is scanning all "Form" subnamespaces of all active bundles for form types. These types could then be accessed by name without declaring them as service.

stof · 2012-12-13T19:16:17Z

@bschussek but what if some of these types have dependencies ? It should probably check first if there is already a tagged service defined for them

webmozart · 2012-12-13T19:30:47Z

@stof If they have dependencies, they obviously cannot be auto-discovered.

…sing of type instances (webmozart) This PR was merged into the 2.8 branch. Discussion ---------- [Form] Deprecated FormTypeInterface::getName() and passing of type instances | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #5321, #15008 | License | MIT | Doc PR | TODO #### Type Names This PR deprecates the definition of the `getName()` method of form types. See #15008 for a more detailed description. Before: ```php class MyType extends AbstractType { public function getName() { return 'mytype'; } // ... } ``` After: ```php class MyType extends AbstractType { // ... } ``` You should always reference other types by their fully-qualified class names. Thanks to PHP 5.5, that's easy: Before: ```php $form = $this->createFormBuilder() ->add('name', 'text') ->add('age', 'integer') ->getForm(); ``` After: ```php $form = $this->createFormBuilder() ->add('name', TextType::class) ->add('age', IntegerType::class) ->getForm(); ``` #### Type Instances Furthermore, passing of type instances is deprecated. Before: ```php $form = $this->createForm(new AuthorType()); ``` After: ```php $form = $this->createForm(AuthorType::class); ``` #### DIC Aliases When registering a type in the DIC, you should omit the "alias" attribute now. Before: ```xml <service id="my.type" class="Vendor\Type\MyType"> <tag name="form.type" alias="mytype" /> <argument type="service" id="some.service.id" /> </service> ``` After: ```xml <service id="my.type" class="Vendor\Type\MyType"> <tag name="form.type" /> <argument type="service" id="some.service.id" /> </service> ``` Types without dependencies don't need to be registered in the DIC as they can be instantiated right away. #### Template Block Prefixes By default, the class name of the type in underscore notation minus "Type" suffix is used as Twig template block prefix (e.g. `UserProfileType` => `user_profile_*`). If you want to customize that, overwrite the new `getBlockPrefix()` method in your type: ```php class UserProfileType extends AbstractType { public function getBlockPrefix() { return 'profile'; } // ... } ``` Commits ------- 3d9e5de [Form] Deprecated FormTypeInterface::getName() and passing of type instances

webmozart mentioned this issue Aug 5, 2014

Added explicit implementation of AbstractType::getName() #11185

Closed

webmozart changed the title ~~[Form] Replace passing type instances by class names~~ [3.0] [Form] Replace passing type instances by class names Oct 16, 2014

webmozart changed the title ~~[3.0] [Form] Replace passing type instances by class names~~ [Form] Replace passing type instances by class names Jun 24, 2015

webmozart mentioned this issue Jun 24, 2015

[Form] Deprecated FormTypeInterface::getName() and passing of type instances #15079

Merged

fabpot closed this as completed Aug 1, 2015

Tobion mentioned this issue Jan 28, 2016

FormType dependency injection #17592

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Form] Replace passing type instances by class names #5321

[Form] Replace passing type instances by class names #5321

webmozart commented Aug 22, 2012

henrikbjorn commented Aug 22, 2012

willdurand commented Aug 22, 2012

webmozart commented Oct 7, 2012

Tobion commented Nov 27, 2012

beberlei commented Nov 27, 2012

stof commented Nov 27, 2012

webmozart commented Dec 13, 2012

webmozart commented Dec 13, 2012

webmozart commented Dec 13, 2012

stof commented Dec 13, 2012

webmozart commented Dec 13, 2012

[Form] Replace passing type instances by class names #5321

[Form] Replace passing type instances by class names #5321

Comments

webmozart commented Aug 22, 2012

henrikbjorn commented Aug 22, 2012

willdurand commented Aug 22, 2012

webmozart commented Oct 7, 2012

Tobion commented Nov 27, 2012

beberlei commented Nov 27, 2012

stof commented Nov 27, 2012

webmozart commented Dec 13, 2012

webmozart commented Dec 13, 2012

webmozart commented Dec 13, 2012

stof commented Dec 13, 2012

webmozart commented Dec 13, 2012