Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Serializer] Add an option to skip null values#28661
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
ostrolucky left a comment
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.
Correct me if I am wrong, but utilizing $context for this most likely means you can't configure Serializer to enable this option by default? If somebody decides to go with this, they will want this to be enabled for whole instance by default. At least that happened in our case. App developer told us he does not want us to send null values and we could easily do that globally, since we used JMS serializer.
dunglas commentedOct 1, 2018
@ostrolucky you're right. I'll also add a constructor option. |
dunglas commentedOct 2, 2018
@ostrolucky done I also widened the visibility of |
| * @var callable|null | ||
| */ | ||
| private$maxDepthHandler; | ||
| protected$maxDepthHandler; |
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.
should be private
| /** | ||
| * @var bool | ||
| */ | ||
| protected$skipNullValues =false; |
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.
shouldbe private also (and the typehint looks useless to me, isn't it?)
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.
As it's an abstract class, making it private allows to reuse this property in child classes... and we need to do it in API Platform.
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.
that's still possible to interact with by decorating the skipNullValues method
protected properties are very hard to deprecate, that's why we really prefer not introducing new ones
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.
We need to access to the value too... in the hot path.
But I agree with your point, and it's why we need to improve the general design of this component.
As a compromise, can we keep it protected but mark it@internal? We'll use it "as is" in API Platform, and if it breaks at some point... we'll upgrade!
dunglas commentedOct 2, 2018
Last commit has been reverted, after discussing with@nicolas-grekas we had a better idea: we'll introduce soon an option to be able to easily set a default context. |
nicolas-grekas commentedOct 2, 2018
We just talked with@dunglas on Slack: he's going to make another PR that will provide a |
0e158f7 tod3c5055Comparenicolas-grekas commentedOct 2, 2018
Thank you@dunglas. |
This PR was squashed before being merged into the 4.2-dev branch (closes#28661).Discussion----------[Serializer] Add an option to skip null values| Q | A| ------------- | ---| Branch? | master| Bug fix? |no| New feature? | yes <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets | n/a <!-- #-prefixed issue number(s), if any -->| License | MIT| Doc PR | todoAdds a new option to not serialize `null` values:```php$dummy = new class { public $foo; public $bar = 'notNull';};$normalizer = new ObjectNormalizer();$result = $normalizer->normalize($dummy, 'json', ['skip_null_values' => true]);// ['bar' => 'notNull']```This feature is the only missing part to add [JSON Merge Patch](https://tools.ietf.org/html/rfc7386) support in [API Platform](https://api-platform.com).It will also help supporting this RFC in all other projects using the Symfony Serializer.Commits-------d3c5055 [Serializer] Add an option to skip null values
This PR was merged into the master branch.Discussion----------[Serializer] Add an option to skip null valuessymfony/symfony#28661Commits-------6349bd3 [Serializer] Add an option to skip null values
Uh oh!
There was an error while loading.Please reload this page.
Adds a new option to not serialize
nullvalues:This feature is the only missing part to addJSON Merge Patch support inAPI Platform.
It will also help supporting this RFC in all other projects using the Symfony Serializer.