Skip to content
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

Avoid repeated information when creating a form type extension #22833

Closed
javiereguiluz opened this Issue May 21, 2017 · 11 comments

Comments

Projects
None yet
6 participants
@javiereguiluz
Copy link
Member

commented May 21, 2017

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version -

If you follow this article https://symfony.com/doc/current/form/create_form_type_extension.html you'll see that:

  1. In the ImageTypeExtension class, you must define which type are you extending with the getExtendedType() method.
  2. When defining a service, you must repeat that information in the extended_type attribute of the form.type_extension tag.

I have two proposals:

  1. Why repeat the information in the tag? Please remove this requirement. Thanks!
  2. I guess there will be some reasons for this but, why must I define the extended type in the getExtendedType() method instead of just extending the class of that type?
@xabbuh

This comment has been minimized.

Copy link
Member

commented May 22, 2017

I guess there will be some reasons for this but, why must I define the extended type in the getExtendedType() method instead of just extending the class of that type?

We are talking about form type extensions here. Making those classes extend some concrete form type class means that they would be implementors of the FormTypeInterface. Thus, an extension would be usable as a form type. That does not make much sense as the goal of a form type extension is not to create new types, but to add functionality to an already existing type.

@javiereguiluz

This comment has been minimized.

Copy link
Member Author

commented May 22, 2017

In PHP we use class extension to add new functionality or change the parent functionality. Since these form things are PHP classes and since this is about adding new functionality, I assumed we could use the natural way here (extending PHP classes) 😢

@jvasseur

This comment has been minimized.

Copy link
Contributor

commented May 22, 2017

@javiereguiluz you can have more than one extension for one form type, that's not possible by extending classes.

@vudaltsov

This comment has been minimized.

Copy link
Contributor

commented May 26, 2017

What if the getExtendedType() method is ignored and deprecated, but multiple form.type_extension tags are supported to add the same extension to multiple form types?

@sstok

This comment has been minimized.

Copy link
Contributor

commented May 26, 2017

@vudaltsov That's not possible as tags are only for type (extensions) registered as services (which is only required when there is a constructor with required parameters).

@sstok

This comment has been minimized.

Copy link
Contributor

commented May 26, 2017

You could make the getExtendedType static, but this will limit use-cases were a type is registered with a configurable parent.

FYI, PHPStorm 2017.2 EAP allows class type completion in string literals (not tried this yet).

@vudaltsov

This comment has been minimized.

Copy link
Contributor

commented May 26, 2017

@sstok , I meant this:

services:
    my_form_extension:
        class: AppBundle\Form\TypeExtension\MyFormTypeExtension
        tags:
            - { name: form.type_extension, extended_type: ...\TextType }
            - { name: form.type_extension, extended_type: ...\TextareaType }
@wouterj

This comment has been minimized.

Copy link
Member

commented May 26, 2017

getExtendedType cannot be removed. It would mean that the form component would be unusable without the DI component. Components should work standalone.

Extending a class to add functionality is not really good OO...

The only thing that can be changed is making getExtendedType() static, so the compiler can call it without initializing the service.

@javiereguiluz

This comment has been minimized.

Copy link
Member Author

commented May 28, 2017

It seems "impossible" to change this. So let's close it as "won't fix". Thanks!

@wouterj

This comment has been minimized.

Copy link
Member

commented May 28, 2017

This solution seems possible? (except that we have to rename the method, so we can make it static without any PHP warning)

The only thing that can be changed is making getExtendedType() static, so the compiler can call it without initializing the service.

@xabbuh

This comment has been minimized.

Copy link
Member

commented Oct 12, 2017

let's see if we can achieve this with #24530

fabpot added a commit that referenced this issue Oct 10, 2018

feature #24530 [Form] simplify the form type extension registration (…
…xabbuh)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[Form] simplify the form type extension registration

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22833, #27906
| License       | MIT
| Doc PR        |

Commits
-------

6a1d4c5 simplify the form type extension registration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.