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

StaticPropertyMetadata first constructor argument not nullable #1116

Closed
KDederichs opened this issue Aug 20, 2019 · 4 comments
Closed

StaticPropertyMetadata first constructor argument not nullable #1116

KDederichs opened this issue Aug 20, 2019 · 4 comments

Comments

@KDederichs
Copy link

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no

Steps required to reproduce the problem

  1. Create a post serialisation listener
  2. Try to add a property according to
    * @deprecated Use visitProperty(new StaticPropertyMetadata(null, 'name', 'value'), null) instead

Expected Result

  • It serialises the property correctly

Actual Result

  • TypeError: Argument 1 passed to JMS\Serializer\Metadata\StaticPropertyMetadata::__construct() must be of the type string, null given

Looks like the first argument is typed as not nullable string, if that value can indeed have the value null the typing in the constructor of StaticPropertyMetadata should be changed.

@goetas
Copy link
Collaborator

goetas commented Aug 20, 2019

This has been updated in https://github.com/schmittjoh/serializer/blob/master/UPGRADING.md

Removed deprecated method JsonSerializationVisitor::addData, use ::visitProperty(new StaticPropertyMetadata('', 'name', 'value'), null) instead

That file has been updated, most probably we forgot to update the comment in the file.

@KDederichs
Copy link
Author

Ok thanks for pointing that out, I do have a follow up question regarding that though:

when I do $visitor->visitProperty(new StaticPropertyMetadata('', 'field', 'value') , null);
It turns out to serialise as null though instead of value

When I do $visitor->visitProperty(new StaticPropertyMetadata('', 'field', 'value' ), 'value2');

It put's value2 in the serialised data.
That behaviour is completely different from both Documentations and I'm wondering if I'm missing something here.

@goetas
Copy link
Collaborator

goetas commented Aug 20, 2019

If you want to update the documentation/comment with the current behavior (that most probably is the one I had in mind when implementing the feature), a PR is welcome

@goetas
Copy link
Collaborator

goetas commented Aug 23, 2019

see #1118

@goetas goetas closed this as completed Aug 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants