symfony: [Form] Form::getErrors() method is confusing for new symfony users
| Q | A |
|---|---|
| Bug report? | no |
| Feature request? | yes |
| BC Break report? | no |
| RFC? | yes |
| Symfony version | 4.0 |
Several times now I saw new inexperienced symfony users getting confused by the Form::getErrors() method. The problem is the boolean $deep parameter with a false default value. Usually it goes like this:
$form->isValid()returns false. Wait what’s wrong?- Oh I will call
$form->getErrors()to see the problem! - It’s empty. If there is no error why is the form not valid?
And here they get stuck for half an hour or more. Of course there is an error but you’ll only see it with $form->getErrors(true). I confess that this took me a while to figure out as well when I was new to Symfony. Boolean parameters are just evil.
To remove this confusion I suggest to deprecate the getErrors method in favor of two new methods getAllErrors and getOwnErrors. I can send a PR if this change is wanted.
About this issue
- Original URL
- State: closed
- Created 6 years ago
- Reactions: 69
- Comments: 21 (15 by maintainers)
IHMO the actual behaviour of
getErrors()(without parameters) is good because the method returns the errors of the current form field. In addition theFormclass has the $errors property so the attened result ofgetErrors()is this property.The real problem of beginners is that they don’t understaind that a form is a tree of fields and not a flat array of fields. Maybe it is a good moment for them to realize that fact when they face the result of
getErrors()being an empy array ?Finding errors via profiler bundle is a workaround. It doesn’t change the fact current behaviour of that method is confusing. Also, people use Form component outside of symfony framework too.
Just my $0.02 here: I’d suggest changing the name of the method from
getErrorstogetErrorIteratoror something similar. My greener self and other new Symfony dev’s I’ve worked with make the mistake of thinking that$form->getErrors()is going to return some kind of array of errors that will be simple to dump / analyze, but instead you get another object with specialized methods you need to call.The name change would work well with the deprecation and switch of the default values as well.
@yceruto You mean as first step deprecating
getErrorscall without parameter and later on allowing it again with opposite default value? Yeah I think that might be possible as well.yes
This was “true” before, but now the debug bar contains a form and a constraints panel. Beginners should learn to use the debug bar. The doc is also OK: https://symfony.com/doc/current/components/form.html#accessing-form-errors
…Removed…
Updated on 2019-05-05: It seems you can just use the
jsonmethod from theControllerTraitto output error information. (for it internally call Symfony serializer)In controller:
Yes
@ro0NL The whole point of this issue is the fact that it does NOT convey that meaning at all. Read the initial post. I do agree that
getErrorsRecursivesharing the same prefix has some benefit but I don’t think it’s enough in this case.There are 2 competing PRs now, we should close one 😃
I like to suggest current
getErrors()+ a newgetErrorsRecursive($flatten = true). IMHOgetErrors()already conveys the meaning of “own errors”, which happens to be the default behavior.getErrorsRecursive()benefits from autocompletion discovery, by sharing a common prefix with the current method. And IMHO is self explanatory.