[Serializer] Remove support for returning empty, iterable, countable, raw object when normalizing #42699
Conversation
… raw object when normalizing
221dd3f to
bf71111
Compare
| switch (true) { | ||
| case ($context[AbstractObjectNormalizer::PRESERVE_EMPTY_OBJECTS] ?? false) && \is_object($data): | ||
| if (!$data instanceof \ArrayObject) { | ||
| trigger_deprecation('symfony/serializer', '5.4', 'Returning empty object of class "%s" from "%s()" is deprecated. This class should extend "ArrayObject".', get_debug_type($data), __METHOD__); |
There was a problem hiding this comment.
Re-reading, I think we should return the original object when it extends ArrayObject
There was a problem hiding this comment.
I disagree. It produces really bad behavior that I explained in your PR.
See the current diff on the test suite to understand.
- "g1":{"list":{"list":[]},"settings":[]},"g2":{"list":["greg"],"settings":[]}}';
+ "g1":{"list":{},"settings":[]},"g2":{"list":["greg"],"settings":[]}}';before: There is an inner list when the list is empty, but it does not exist when the list is not empty
after: the shape is always the same, whereas the list is empty or not
=> This is a bug fix
There was a problem hiding this comment.
That's a behavior change that we'd better not do imho. The type hint doesn't require this.
Or we'd need another deprecation on 5.4, but I don't see why we'd do this (especially since there is a test about this).
There was a problem hiding this comment.
I added this test BTW, to show how it behaves.
I thought it was clear in the previous conversation... 😬
I can do what you want, but I persist to say it's a bad idea, since the serializer is not doing what people expect!
There was a problem hiding this comment.
See #42730 as follow up of this discussion.
…jects when PRESERVE_EMPTY_OBJECTS is set (nicolas-grekas) This PR was merged into the 5.4 branch. Discussion ---------- [Serializer] Return an ArrayObject for empty collection objects when PRESERVE_EMPTY_OBJECTS is set | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - In #42619, we tried adding a BC layer for this behavior change, but when reviewing #42699, I figured out there is no possible BC layer: userland implementations (eg an entity returning an empty doctrine collection) won't extend ArrayObject because we ask them to do so. I therefor propose to return an ArrayObject as of 5.4, relying on the intuition that this should not have much BC impact in practice. /cc `@lyrixx` Commits ------- 7856fe7 [Serializer] Return an ArrayObject for empty collection objects when PRESERVE_EMPTY_OBJECTS is set
The test suite is now green 👍
ping @nicolas-grekas