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

ValuesJsonConverter doesn't pass JsonSerializer through to ToObject #104

Closed
Turnerj opened this issue Dec 8, 2019 · 1 comment · Fixed by #105
Closed

ValuesJsonConverter doesn't pass JsonSerializer through to ToObject #104

Turnerj opened this issue Dec 8, 2019 · 1 comment · Fixed by #105
Labels
bug Issues describing a bug or pull requests fixing a bug.

Comments

@Turnerj
Copy link
Collaborator

Turnerj commented Dec 8, 2019

The ToObject calls on JObject or JToken etc inside the ValuesJsonConverter don't pass the current serializer. This is a problem as custom converters on the serializer are never passed through (eg. StringEnumConverter) to the inner deserialization of the token/object.

This would affect all nested properties that have IThing (or anything derived from it) as a potential value.

The fix is basically just passing the instance of JsonSerializer through to the ToObject method calls within ValuesJsonConverter.

I would add a specific way to reproduce this however I came into this issue from a different/complicated angle. I was trying to deserialize a BreadcrumbList where the list item's "item" field wouldn't deserialize because it was an IThing and there was no defined type. When I tried to use my own converter (basically taking the IThing the serialization process saw and making it instantiate Thing), I saw my converter was never passed down the line into the nested properties.

I'm not sure if it is worth fixing for JSON.Net or just something to keep in mind for #100 .

@Turnerj Turnerj added the bug Issues describing a bug or pull requests fixing a bug. label Dec 8, 2019
@RehanSaeed
Copy link
Owner

I'm wondering if we can switch to #100 which is fixed but still work in progress. Lets see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug or pull requests fixing a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants