[DX] Call $form->createView() automatically

[webmozart]

A mistake that I frequently see is that people forget to call createView() when passing a form to the view. What do you think about converting FormInterface instances to FormView objects automatically by calling createView(), for example, in a listener?

[javiereguiluz]

In terms of code and pure DX, a big 👍 for this feature. My only concern is in terms of performance. Can we estimate, at least in a roughly manner, the performance hit that would be introduced by that listener?

[stof]

There os no listener hooking in the call to Twig done by the controller.

So this may work for people using the @Template annotation of FrameworkExtraBundle to rely on the kernel.view listener, but I don't see how this could work in core.

[mmoreram]

Agree with @stof.

@webmozart what the listener should be prepared to change? The array returned by the controller? So, how do you know the variable to modify? And if the user just want to pass the FormView? Has no sense, sure, but what if?

Edit: BTW, that means this listener should parse all the array checking all the types?

Maybe too much magic. I think is just something the framework or any component should be aware of, is responsibility of the developer to know the specification of what is he working with.

[stof]

@mmoreram looping over variables passed to the template and checking whether they are instanceof FormInterface. If you pass a FormView directly, we would do nothing.

[raulfraile]

I also think this would be a big improvement from the point of view of DX. Regarding the implementation, I don't see how it could be done in the core, but as @stof suggested, the @Template annotation of SensioFrameworkExtraBundle seems to me like a good way to do it, so only checks for FormInterface instances in the controller response if configured that way:

/**
 * @Template(convertForms=true)
 */
public function newAction()
{
    return array(
        'form' => $form
    );
}

[stof]

Maybe too much magic. I think is just something the framework or any component should be aware of, is responsibility of the developer to know the specification of what is he working with.

Well, this is a suggestion to make it easier to use

[catchamonkey]

@template(convertForms=true)

If you have to configure it in the Template annotation, then I don't think that is as helpful in the DX sense as the developer would still have to remember to do it each time.

[mmoreram]

@catchamonkey +1

[xabbuh]

So this may work for people using the @template annotation of FrameworkExtraBundle to rely on the kernel.view listener, but I don't see how this could work in core.

This could be done in the renderView() method of the base controller from the FrameworkBundle. So, it would at least be available to all developers extending this base class.

[docteurklein]

Could we delegate the transformation to the form_* helpers ?
I don't think so, since it would create a new FormView instance at each call. (thinking loud).

Otherwise, there is something similar in FOS/RestBundle, just for you to know. (again, only works with kernel.view event)

[stof]

Could we delegate the transformation to the form_* helpers ?
I don't think so, since it would create a new FormView instance at each call. (thinking loud).

This would indeed not work. The rendering must use a single instance, not create a new one each time

[mmoreram]

IMHO any component should modify the controller return data. A nice workaround just for debug mode would be adding a notice or a log line when a FormType is detected in the controller return array.

[stof]

FormType ? If you return a FormType, you have a much bigger issue, as you already called non-existent methods previously when binding the form

[mmoreram]

@stof Sorry, Form. "Mondays effect"

[mickaelandrieu]

👎 for the same @mmoreram 's reasons.

[wouterj]

I like the idea, calling createView always seemed like a redundant call. However, we have to redesign how this works, since there is no way to do this.

I see however other features which could use this new refactoring. For instance, creating templates online using the available variables, debugging templates better, etc.

[filipgorny]

Hi guys,

please look at my proposition how to refactor it. It work's fine for me and allow to either call createView or not.

filipgorny@2605b0c

I believe the need to call createView comes from some missleading design. The view creation should be moved away from Form itself.

However I am not sure if it won't violate any other components compatibility.

[stof]

@filipgorny this does not work. If you render the form by calling form_start, form_widget and form_end explicitly rather than form, or if you render each child explicitly (well, any combination involving several renderer calls), you will create a new FormView for each rendering call, breaking everything (it is very likely that the same form input will be rendered several times by different FormView trees).
The FormView must be created before doing the rendering.

[filipgorny]

@stof Well it is created (or not) before the rendering, if the FormView instance is passed, it's createView method returns this instance, so infact there is no change. FormView initialization is only in the Form class.

[stof]

@filipgorny it is not created before the rendering. It is called inside the rendering. If form is a form instance in the following code, you will create 3 different FormView trees (one for each renderer call):

{{ form_start(form) }}
{{ form_widget(form) }}
{{ form_end(form) }}

it becomes even worse for the case where you render fields explicitly, as you will get duplicate rendering (because the explicit rendering will be done for a separate FormView tree)

[filipgorny]

@stof Now I get it. The only idea I have now is to cache the result of createView but it doesn't sound satisficing.

[stof]

no, $form->createView() needs to be usable several times. Changing this would be a BC break.
This is exactly why we suggested doing this automatic handling in a different layer

[peterrehm]

One idea would be to check the parameters passed to twig and convert FormInterface to FormView. This should actually easy to implement.

[docteurklein]

@peterrehm how would you do that ? with a custom template engine, that wraps default one ?

[peterrehm]

@docteurklein I think the twig render command needs to be adjusted either with wrapping it or with including it into twig and also the Symfony\Bundle\FrameworkBundle\Controller\Controller:render() method needs to be adjusted. There we could check the parameters and if an Form instance is provided convert it to the FormView

[docteurklein]

No, you can render forms from somewhere else than controllers and using other engines than twig.

Actually it's a little bit more complicated (if we go with wrapping EngineInterfaces), since we need to support EngineInterface, StreamingEngineInterface and FrameworBundle\EngineInterface (and look how it deals with DelegatingEngine.

Well, at the end it would be basic delegation.
I'd like to have your thoughts about that @stof ?

[linaori]

What about the fact that you return a FormType but have a FormView in your template? That can become very confusing, very fast imo. How about creating a generic interface that you can use which has a createView for example? The TemplateListener could then create a view variant of the objects:

// just to give an example
foreach ($template_vars as &$var) {
    if ($var instanceof ViewTransformer) { // or what ever name seems fit
        $var = $var->createView();
    }
}

Next to making this slightly user friendly, it becomes easier to document and makes it reusable.

Edit: I would go in the SensioFrameworkExtraBundle: https://github.com/sensiolabs/SensioFrameworkExtraBundle/blob/master/EventListener/TemplateListener.php

[cordoval]

this can be optional in a package, not in core as symfony is not about magic or at least that is what their foundations were (we quickly forget), to escape symfony 1.x magic nightmare, but it is a nice package/bundle for perhaps a SE-DX distribution. So mixed feelings 👎 and 👍 for DX, I am surprised also not seeing something like this in https://github.com/KnpLabs/KnpRadBundle

[nifr]

Just as an indicator how often people seem to stumble over this ...

stackoverflow question, 1 year, ~3000 views

[mvrhov]

The FOSRest bundle solves this nicely. It automatically calls createView on data key named 'form' which is instanceof Form

[linaori]

@mvrhov I don't call that nicely, that's convention magic.

[stof]

Well, the Controller shortcut method could iterate over values passed in the context and call createView on them for FormInterface. The TemplateListener in FrameworkExtraBundle could do the same.
This way, only people calling the templating engine directly will need to remember they need to create the view.

[aderuwe]

@iltar checking the type of something and then calling a method supported by that type is hardly convention magic.

[stof]

checking the type of something and then calling a method supported by that type is hardly convention magic.

@aderuwe the fact of doing it only for the variable named form is.

[aderuwe]

@stof You are right, I missed that in the message I replied to. Must be for performance on larger forms? More limiting than I had expected ... but I guess I'm now off-topic.

[GromNaN]

I like @mmoreram's idea of advising developers when they pass a form to the view.

A nice workaround just for debug mode would be adding a notice or a log line when a Form is detected in the controller return array.

Checking every parameter to occasionally convert one of them seems overloading ; especially on prod environment.

[linaori]

@GromNaN https://github.com/sensiolabs/SensioFrameworkExtraBundle/blob/master/EventListener/TemplateListener.php#L90

Parameters already checked here if you use the view. Adding an instanceof check won't kill your website and if you take a look to my piece of code a few replies above, you will see how easy it can be.

[fabpot]

Closing as there is apparently no way to implement this in a generic way that would work well in all cases.