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

Support php7.4 nullable typed properties for JMS serializer. #2103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zviryatko
Copy link

In the case of php7.4 typed properties for JMS serializer there is no difference between nullable typed property and required one:

    /**
     * @Serializer\Type("integer")
     */
    private int $id;

    /**
     * @Serializer\Type("string")
     */
    private ?string $name;

Spec is generated for both properties as optional, but the $id must be required.

This PR adds support for marking fields as required automatically for php7.4 typed properties and for virtual properties based on their return type's nullability.

@zviryatko
Copy link
Author

Oops, it seems I need to move it out to a separate controller to avoid conflicts with <7.4 tests.

@zviryatko zviryatko force-pushed the jms_serilizer_nullable_typed_properties branch from c4cce49 to ae1b7a6 Compare May 26, 2023 11:39
@zviryatko
Copy link
Author

It seems master branch is corrupted, so waiting for fix upstream, later I will rebase.

@shakaran
Copy link
Contributor

shakaran commented May 1, 2024

@zviryatko could you rebase with last updates? Thanks

@zviryatko zviryatko force-pushed the jms_serilizer_nullable_typed_properties branch 3 times, most recently from e9a1979 to 323eee4 Compare May 1, 2024 19:34
@zviryatko zviryatko force-pushed the jms_serilizer_nullable_typed_properties branch from 323eee4 to 0bee583 Compare May 1, 2024 20:36
@zviryatko
Copy link
Author

@shakaran finally did that 🙄

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

Successfully merging this pull request may close these issues.

None yet

2 participants