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

[System.Test.Json] JsonStringEnumConverter should support enum values marked with EnumMember and use the value from that when serializing/deserializing. #36931

Closed
AraHaan opened this issue May 23, 2020 · 4 comments

Comments

@AraHaan
Copy link
Member

AraHaan commented May 23, 2020

I personally think that EnumMember should be supported, many users try to use it with the JsonStringEnumConverter that comes shipped with System.Text.Json itself. Because of this they can run into an System.Text.Json.JsonException from doing such which sucks.

Take a look at the draft pr that is on hold because this is 1 of the issues they face when trying to eradicate newtonsoft.json: RehanSaeed/Schema.NET#100

Also take a look at a version of my code which did that, then I have to now struggle trying to get my stuff to compile because now I got to create a string extension method that could allow null strings somehow and then filter it to the enum values accordingly manually which also sucks.
https://paste.mod.gg/giwezarimo.cs
https://paste.mod.gg/nuqofuwamu.cs
https://paste.mod.gg/gunoqemufu.cs

You can literally use this code as a minimal example and as you can tell my code will not compile as this stuff is lacking. Note: I only made this change because of that pull request above I seen that it used EnumMember and that it used a converter I did not really know about so I thought "Wait a second so my manual converters is not needed at all?"

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels May 23, 2020
@jmaine
Copy link
Contributor

jmaine commented May 25, 2020

This looks like an easy issue to fix. The only question is "which version do we want to put the fix in?"

@jmaine
Copy link
Contributor

jmaine commented May 25, 2020

I am working on a pull request for this.

@AraHaan
Copy link
Member Author

AraHaan commented May 26, 2020

Most likely the master branch.

@layomia layomia added this to the 5.0 milestone May 26, 2020
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label May 26, 2020
@layomia
Copy link
Contributor

layomia commented May 26, 2020

This is a dup of #31081.

@layomia layomia closed this as completed May 26, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants