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

[Form] BC break (maybe?) between v3.4.20 and v3.4.21 #29841

Closed
simshaun opened this Issue Jan 10, 2019 · 5 comments

Comments

Projects
None yet
5 participants
@simshaun
Copy link
Contributor

simshaun commented Jan 10, 2019

Symfony version(s) affected: v.3.4.21

Description
Caused by #29307 (/cc @nicolas-grekas)

I've got a MainFormType that contains a collection of SubFormType. SubFormType has a hidden field that holds a JSON-encoded string (generated by JS in my actual use case). SubFormType transforms the string to an array on submit.

Everything has been working fine until 3.4.21 when the form started giving errors. I'm not really sure if this is a BC break or not, but it has definitely caused us some problems.

How to reproduce
I've created a repro branch at https://github.com/simshaun/symfony-standard/tree/form_bc_break_3.4.21

  • Run phpunit to confirm errors.
  • Downgrade to Symfony 3.4.20 and run phpunit again.
@xabbuh

This comment has been minimized.

Copy link
Member

xabbuh commented Jan 10, 2019

looks like the same as #29809

@simshaun

This comment has been minimized.

Copy link
Contributor

simshaun commented Jan 10, 2019

Hm maybe. The difference being, instead of submitting an array structure I'm submitting a string then transforming it to an array in a form event listener. I guess both ways ultimately look the same to the form component. It's frustrating when a bugfix (was the scalar data issue really a bug or just a bad unintended feature?) causes problems like this.

For anyone else running into this problem, drop the PRE_SUBMIT form event listener and add a model transformer, e.g. below:

$builder->add(
    $builder->create('settings', HiddenType::class)
        ->addModelTransformer(new JsonTransformer())
);
<?php

namespace SomeRandomNamespace;

use Symfony\Component\Form\DataTransformerInterface;

class JsonTransformer implements DataTransformerInterface
{
    public function transform($data)
    {
        if (null === $data) {
            return null;
        }

        return json_encode($data);
    }

    public function reverseTransform($json)
    {
        if (!$json) {
            return null;
        }

        return json_decode($json, true);
    }
}
@nicolas-grekas

This comment has been minimized.

Copy link
Member

nicolas-grekas commented Jan 11, 2019

It worked by chance - and it made any field accept arrays of values by just appending [] to the name of form inputs. Definitely not the definition of proper input filtering.
Using a data transformer is a nice idea - and a much more reliable one, thanks for sharing.

@stof

This comment has been minimized.

Copy link
Member

stof commented Jan 11, 2019

Well, using a PRE_SUBMIT event listener means that the HiddenType receives an array as submitted value, while it expects a string as it data. The fact that it worked before was just luck (as it does not do anything special with its data except passing it around, it kind of worked). Btw, in case of an invalid form being re-displayed, it might even have been broken (as the template would then have to render the array) so I'm not even sure it fully worked before.

Using a model data transformer is exactly how your use case should be implemented.

@xabbuh

This comment has been minimized.

Copy link
Member

xabbuh commented Jan 11, 2019

I am closing here as this is not a bug as explained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment