Skip to content
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

[FrameworkBundle] Enable json_decode_detailed_errors in dev by default #51215

Merged
merged 1 commit into from Aug 3, 2023

Conversation

ostrolucky
Copy link
Contributor

@ostrolucky ostrolucky commented Aug 1, 2023

Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #51172 (comment)
License MIT
Doc PR

Follows

@ostrolucky ostrolucky force-pushed the auto-enable-json-detailed-errors branch 4 times, most recently from 2a551c2 to 4939f03 Compare August 2, 2023 10:23
@ostrolucky ostrolucky force-pushed the auto-enable-json-detailed-errors branch from 4939f03 to b595e90 Compare August 2, 2023 10:32
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works because performNoDeepMerging is not set on the node, OK.

@fabpot
Copy link
Member

fabpot commented Aug 3, 2023

Thank you @ostrolucky.

@fabpot fabpot merged commit 08b93ad into symfony:6.4 Aug 3, 2023
3 of 9 checks passed
This was referenced Oct 21, 2023
@@ -1123,6 +1125,10 @@ private function addSerializerSection(ArrayNodeDefinition $rootNode, callable $e
->arrayNode('default_context')
->normalizeKeys(false)
->useAttributeAsKey('name')
->beforeNormalization()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding that default in beforeNormalization makes it very hard to re-disable the feature because each source will re-enable it in this normalization (well, each source configuring the default_context).
This should be done in the validate phase instead.

->beforeNormalization()
->ifTrue(fn () => $this->debug && class_exists(JsonParser::class))
->then(fn (array $v) => $v + [JsonDecode::DETAILED_ERROR_MESSAGES => true])
->end()
->defaultValue([])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default value should probably also include JsonDecode::DETAILED_ERROR_MESSAGES => true (when appropriate) so that this configuration also applies when no default_context is configured at all.

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.

None yet

7 participants