Port JsonIsomorphism from Newtonsoft variant #40
Conversation
| if baseConverter |> isNull then | ||
| failwithf "Field %s is decorated with a JsonConverter attribute, but it does not implement a CreateConverter method." f.Name | ||
|
|
||
| let baseConverter = |
There was a problem hiding this comment.
I inferred this makes sense based on a) reading the xmldoc for JsonConverter in STJ b) the tests passing c) reading the converter authoring guide a bit
Seeking a review as its entirely possible I'm missing lots of obvious things...
There was a problem hiding this comment.
I think it would work, but the question is should this be something that is supported? If someone creates JsonConverterAttribute without implementing the CreateConverter method, then it's essentially an invalid use of the attribute. I don't know if we should go around that and try to create the converter anyways.
There was a problem hiding this comment.
If it was mandatory, surely it'd be abstract?
For me, it seems the docs and xmldocs suggest that they don't all need to be converter factories
And letting it work direct seems in some cases to avoid an allocation, which would be in line with the general STJ philosophy
I guess we'll leave this here so someone can like to it in a told-you-so comment when we discover its wrong!
| if baseConverter |> isNull then | ||
| failwithf "Field %s is decorated with a JsonConverter attribute, but it does not implement a CreateConverter method." f.Name | ||
|
|
||
| let baseConverter = |
There was a problem hiding this comment.
I think it would work, but the question is should this be something that is supported? If someone creates JsonConverterAttribute without implementing the CreateConverter method, then it's essentially an invalid use of the attribute. I don't know if we should go around that and try to create the converter anyways.
Port of original NewtonsoftJson impl by @eiriktsarpalis to work equivalently with
System.Text.Json(separated for now for review purposes, part of #38)