Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[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

Merged
merged 1 commit into from
Sep 28, 2022

Conversation

siganushka
Copy link
Contributor

@siganushka siganushka commented Feb 25, 2022

Fixed framework.serializer.default_context is not working for JsonEncoder

like XmlEncoder or DateTimeNormalizer, the array $defaultContext argument bindings from SerializerPass, but not working for JsonEncoder, I added array $defaultContext argument and passed to JsonEncode and JsonDecode.

https://github.com/symfony/serializer/blob/5.4/DependencyInjection/SerializerPass.php#L69-L75

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no

Copy link
Member

@dunglas dunglas left a 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)
Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Member

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!

@fabpot
Copy link
Member

fabpot commented Jul 21, 2022

@siganushka Are you still interested in moving this PR forward?

@nicolas-grekas
Copy link
Member

Thank you @siganushka.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.