-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Serializer] Fixed framework.serializer.default_context is not working for JsonEncoder #45554
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ad this is a new feature, this should target the 5.c branch. Also, maybe should we allow providing different contexts for encoding and decoding.
@@ -23,10 +23,10 @@ class JsonEncoder implements EncoderInterface, DecoderInterface | ||
protected $encodingImpl; | ||
protected $decodingImpl; | ||
|
||
public function __construct(JsonEncode $encodingImpl = null, JsonDecode $decodingImpl = null) | ||
public function __construct(array $defaultContext = [], JsonEncode $encodingImpl = null, JsonDecode $decodingImpl = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a BC break, could you move this parameter at the end of the list please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, but I'm not sure how to set different defaultContext
for encoding and decoding, because JsonEncode
and JsonDecode
are not passed into JsonEncoder
as a service, and other places are the same defaultContext
set in framework.serializer.default_context
, Like XmlEncoder
or DateTimeNormalizer
.
also I have tried providing the defaultContext
parameter at the end, but this raises an error in DefinitionErrorExceptionPass
:
Invalid constructor argument 3 for service "serializer.encoder.json": argument 1 must be defined before. Check your service definition.
Do you have any idea?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the proper solution is maybe to register them as services indeed!
@siganushka Are you still interested in moving this PR forward? |
…g for JsonEncoder
Thank you @siganushka. |
Fixed
framework.serializer.default_context
is not working forJsonEncoder
like
XmlEncoder
orDateTimeNormalizer
, thearray $defaultContext
argument bindings fromSerializerPass
, but not working forJsonEncoder
, I addedarray $defaultContext
argument and passed toJsonEncode
andJsonDecode
.https://github.com/symfony/serializer/blob/5.4/DependencyInjection/SerializerPass.php#L69-L75