-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
fix: Add array to return of InputBag::get #41766
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
Conversation
This is on purpose. If you want to get an array, you should use |
This is quite the BC break. Is there a place where we can find the reasoning behind this? There is no explanation in the upgrade notes or the commits. |
It was introduced in #34363. You will find some explanations here! ;) |
Only adding the array type will not fix it. the if statement check in the get should be the same as the set function so you should add |
@nicolas-grekas even if it is deprecated, it can still return an array in 5.3 |
Sure, but we don't phpdocument deprecated types. |
@nicolas-grekas we will need to change that policy anyway. With native return types, we cannot remove the deprecated types from the signature, otherwise we get a TypeError |
Only adding the array type in the docblock doesn't fix anything. This is still a bug and throws a error because it doesn't accept array while it accepts it in the set function. if (null !== $value && $this !== $value && !is_scalar($value)) {
throw new BadRequestException(sprintf('Input value "%s" contains a non-scalar value.', $key));
} It should also accept array again because a lot of websites and app are broke now like mine. It should accept the same as in the set function vendor/symfony/http-foundation/InputBag.php:77 if (null !== $value && !is_scalar($value) && !\is_array($value) && !$value instanceof \Stringable) {
throw new \InvalidArgumentException(sprintf('Excepted a scalar, or an array as a 2nd argument to "%s()", "%s" given.', __METHOD__, get_debug_type($value)));
} |
@dvdknaap you need to call now |
But how to test is given key is array or scalar value.
First case you can get only through I can't see a way how to test is |
I also think this leads into some strange cases now specially when working with the FOSRestBundle which is setting the JSON Content to |
This breaks my code guys. Bad form. |
a deprecation breaks your code? |
you mostly know what your application expects getting the raw value is done using |
@ddr5292 And another one, also broke my code on a minor dependency update. For anyone searching for this on the internet, if you get |
Change from $request->query->get to $request->query->all because of Symfony 6 ( symfony/symfony#41766 (comment) ). This change prevent this error: "Input value "horse_filter" contains a non-scalar value."
I cannot agree more with you. I use get() for all type of data. but now it all mess up. The worst thing is that I cannot see the reason why it worth even introducing a breaking change. User submitted value could be very unpredictable. Sometimes it's on purpose and allowed to input either a scalar value or an array. What we need is the flexibility in the back-end to handle it. |
This ship sailed long ago. As you can see, this has been discussed numerous times 2 years ago, so it's unlikely that the outcome changes now. Furthermore, both deprecation and new behavior has been part of Symfony for nearly 4 years. Changing or reverting now is impossible and will impact more applications than changing this in existing applications that are upgrading to 5.x or 6.x now. Please also do not comment more or less the same things across multiple issues/PRs within the same topic. This divides the same discussion across multiple places, making it harder to track. |
I'm locking all of those issues now. If there's something NEW to discuss, please open a new issue. |
The method can return array. The
set
method has already array typehint