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

Convert Newtonsoft converters to STJ converters #14457

Closed
wants to merge 3 commits into from

Conversation

hishamco
Copy link
Member

@hishamco hishamco commented Oct 8, 2023

Addresses part of #14456

@@ -105,9 +77,64 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
return contentItem;
}

public override bool CanConvert(Type objectType)
public override void Write(Utf8JsonWriter writer, ContentItem value, JsonSerializerOptions options)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is might be a temp solution I'm working to add a JTokenConverter

@sebastienros
Copy link
Member

The unit tests seems to fail because an entity has a JObject. This should become a JsonDocument (or JsonElement).

@sebastienros
Copy link
Member

Unless here you are trying to keep the JObject and just use STJ to do the de/serialization which might be harder, though a totally valid approach.

@hishamco
Copy link
Member Author

I might defer ContentItemConverter after we change IEntity

@MikeAlhayek
Copy link
Member

@hishamco is this still needed? If not, please close it

@hishamco
Copy link
Member Author

Nope, also there's another PR from @sarahelsaig need to be deleted too

@hishamco hishamco closed this Dec 24, 2023
@hishamco hishamco deleted the hishamco/json-converters branch December 24, 2023 18:15
@sarahelsaig
Copy link
Contributor

Thanks for pointing it out, Hisham. I closed it.

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.

4 participants