[JsonStreamer] Add number object support #59915
Conversation
| $nativeToStreamValueTransformers = $this->nativeToStreamValueTransformers; | ||
|
|
||
| $nativeToStreamValueTransformers[] = $nativeToStreamValueTransformer; | ||
| $nativeToStreamValueTransformers = array_values(array_unique($nativeToStreamValueTransformers)); |
There was a problem hiding this comment.
Stop trying to make value transformers unique, as it does not add any value, and didn't work anyway because \Closure aren't castable to string.
mtarld
force-pushed
the
feat/json-streamer-number
branch
3 times, most recently
from
003a6f0 to
4926638
Compare
src/Symfony/Component/JsonStreamer/ValueTransformer/ValueObjectToScalarValueTransformer.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/JsonStreamer/ValueTransformer/ScalarToValueObjectValueTransformer.php
Outdated
Show resolved
Hide resolved
mtarld
force-pushed
the
feat/json-streamer-number
branch
from
4926638 to
47788ad
Compare
mtarld
changed the title
mtarld
force-pushed
the
feat/json-streamer-number
branch
from
47788ad to
687104b
Compare
mtarld
force-pushed
the
feat/json-streamer-number
branch
from
687104b to
6a02953
Compare
| use Symfony\Component\TypeInfo\Type\UnionType; | ||
|
|
||
| /** | ||
| * Transforms value object to scalar. |
There was a problem hiding this comment.
| * Transforms value object to scalar. | |
| * Transforms value objects to scalars. |
| * | ||
| * @internal | ||
| */ | ||
| final class ValueObjectTypePropertyMetadataLoader implements PropertyMetadataLoaderInterface |
There was a problem hiding this comment.
should this be split as well for the same reasons (#59915 (comment))?
stof
left a comment
stof
left a comment
There was a problem hiding this comment.
I would suggest splitting the PR to make the review easier. Adding support for union types is totally separate from adding support for the GMP or BCMath objects. It would make the review a lot easier.
| { | ||
| if (!$value instanceof \DateTimeInterface) { | ||
| throw new InvalidArgumentException('The native value must implement the "\DateTimeInterface".'); | ||
| return $value; |
There was a problem hiding this comment.
Why making it return the original value instead of failing when it cannot transform it ?
| $propertyMetadataLoader = new GenericTypePropertyMetadataLoader( | ||
| new DateTimeTypePropertyMetadataLoader( | ||
| $propertyMetadataLoader = new ValueObjectTypePropertyMetadataLoader( | ||
| new GenericTypePropertyMetadataLoader( |
There was a problem hiding this comment.
Why changing the order of decorators ?
|
As suggested by @stof in #59915 (review), I gonna close this PR in favor of two new ones. |
4 participants
Add support to
BcMath\NumberandGMPvalue objects. And, add support for union of value objects and other types (which was not working before).Plus, this PR fixes the order of property metadata loaders decoration to make "attribute property metadata loader" acting first, and "value object property metadata loader" last.