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

[DX] Form Types location contradicts Best Practices #6266

Closed
wants to merge 1 commit into from
Closed

[DX] Form Types location contradicts Best Practices #6266

wants to merge 1 commit into from

Conversation

pbowyer
Copy link
Contributor

@pbowyer pbowyer commented Feb 12, 2016

I wondered why I always put form definitions in AppBundle/Form/Type, and whether we were meant to put something else in AppBundle/Form. While Googling for an answer I came across the Forms Best Practice guide, which says the new best practice is to put the form types (which I guess are usually form definitions, so the change makes sense) directly in AppBundle/Form.

This PR updates the main documentation to reflect the best practices. If anyone can point me to discussions clarifying the difference between form types, form definitions and how the Symfony view has changed (reflected in the change of location) I'd greatly appreciate it.

I wondered why I always put form definitions in `AppBundle/Form/Type`, and whether we were meant to put something else in `AppBundle/Form`. While Googling for an answer I came across http://symfony.com/doc/current/best_practices/forms.html which says the new best practice is to put the form types (which I guess are usually form definitions) directly in `AppBundle/Form`.

This PR updates the [main documentation ](http://symfony.com/doc/current/book/forms.html#book-form-creating-form-classes) to reflect the best practices. If anyone can point me to discussions clarifying the difference between form types, form definitions and how the Symfony view has changed (reflected in the change of location) I'd greatly appreciate it.
@xabbuh
Copy link
Member

xabbuh commented Feb 12, 2016

👍

@wouterj
Copy link
Member

wouterj commented Feb 13, 2016

Hi @pbowyer!

First of all: 👍 I like this a lot. However, we have to merge changes in the oldest branch and then merge them up into the newer ones. As there has been a lot of form refactorings between 2.3 (oldest) and 3.0 (your current branch), I can't make a nice merge. If possible, can you please redo these changes against the 2.3 version of the docs? (if it's not possible, just comment and we'll do it for you)

Then to answer your question: There is no difference between form types and form definitions in Symfony. Each form consists of a root form type, which has children. The children can have children as well, etc. So each field is the end of a branch of this form tree, where node is a form type. This has some big benefits, one of these is that you can reuse "form definitions" in other forms. So an CreateAddressFormType can be used as the root form type when a user adds a new address, but it can also be included in a CreateUserFormType.

The discussion about the location change can be found here: #6059

@theofidry
Copy link
Contributor

whether we were meant to put something else in AppBundle/Form

You can also have transformers (AppBundle/Form/DataTransformer) and extensions (AppBundle/Form/Extension).

That being said, I agree with @weaverryan comment. But the cookbook needs to be updated as well.

@pbowyer
Copy link
Contributor Author

pbowyer commented Feb 17, 2016

Hi @wouterj,

If possible, can you please redo these changes against the 2.3 version of the docs? (if it's not possible, just comment and we'll do it for you)

I've made the changes here, but couldn't work out how to get them into the patch-3 branch to show against this ticket, as that's a 3.0 branch. If you can tell me, I'll do it :)

@wouterj
Copy link
Member

wouterj commented Jun 11, 2016

Hi @pbowyer. I'm very sorry to have completely ignored you to this moment.

Can you maybe create a new pull request with that branch against 2.3? We'll close this one then and things can be merged. Thanks for your work!

@wouterj
Copy link
Member

wouterj commented Jul 8, 2016

Hi @pbowyer! Thanks for creating this PR.

I've rebased this PR to the 2.7 branch in #6733 . I'll merge that PR soon.

@wouterj wouterj closed this Jul 8, 2016
wouterj added a commit that referenced this pull request Jul 8, 2016
…yer)

This PR was merged into the 2.7 branch.

Discussion
----------

[DX] Form Types location contradicts Best Practices

Rebase of #6266

Commits
-------

f9597a6 [DX] Form Types location contradicts Best Practices
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants