Writable Json DOM

[kasiabulat]

AB#1224692
Adding a modifiable API in System.Text.Json to complement the readonly JsonDocumet.

Goals

• Designing API
• Implementation of provided methods
• Tests of provided methods
• Documentation for public API

Specification

https://github.com/dotnet/corefx/blob/master/src/System.Text.Json/docs/writable_json_dom_spec.md

[ahsonkhan]

Here's the work items left to do for this feature (in order/priority):

• Merge this PR: JSON DOM refactor corefx#41041
• React to initial usability study feeback: Rename JsonNode and related types to JTreeNode, move them to S.T.Json.Linq namespace, and address partial usability feedback corefx#41128
• Improve code coverage: https://github.com/dotnet/corefx/issues/41130
• Add support for Base64: https://github.com/dotnet/corefx/issues/41132
• Add performance test and investigate perf gaps: https://github.com/dotnet/corefx/issues/41129
• Improve JsonNumber performance/internal implementation: https://github.com/dotnet/corefx/issues/39957
• Investigate alternative design (making the JsonNode type hierarchy internal to JsonElement and unify the types) and conduct A/B usability study
• Prototype and implement APIs that interop with Utf8JsonReader, Utf8JsonWriter, and JsonSerializer
• API Review and react to any feedback from that
• [might not apply if API shape changes] Implicit cast operators for JsonNumber: https://github.com/dotnet/corefx/issues/40118
• [might not apply if API shape changes] Consider adding AggressiveInlining to JsonNode primitive types equality operator: https://github.com/dotnet/corefx/issues/40186
• [might not apply if API shape changes] Add more tests for different StringComparison values: JSON DOM refactor corefx#41041 (comment)

cc @joperezr

[mrange]

What is the progress on this issue out interest?

[joperezr]

@ahsonkhan seems like the checkboxes that we have above are a bit out of date right? since I believe that at least we have already done the second and third in the list but I may be wrong.

@mrange Once Ahson confirms, the above comment will show the progress, and the milestone hasn't changed so we are still planning on shipping this on for .NET 5.0.

[stephentoub]

@terrajobst, my understanding is we've already checked in APIs here, but it's showing as "api-suggestion". How are we tracking APIs that have been merged but not reviewed?

[joperezr]

@stephentoub given this feature's scope was big and needed to keep in sync with the fast developing System.Text.Json library, we did get special sign-off from the team to work and check-in directly into the master branch for iterations, and to have an API Review at the end in order to flush out any remaining issues we might still have. All that said, it is worth noting that we did have at least 2 design reviews and user studies around this API before it got checked in.

[stephentoub]

That's good to hear, but a) the checklist still lists "React to initial usability study feeback: dotnet/corefx#41128" as not done, b) the associated PR saying it was updating lots of surface area was closed (dotnet/corefx#41128), c) the assigned owner for this issue was someone no longer working on the project, d) part of this issue is "Investigate alternative design (making the JsonNode type hierarchy internal to JsonElement and unify the types) and conduct A/B usability study" which suggests we're not completely satisfied with the shape of things, e) the issue is listed as "api-suggestion" which doesn't jive with the fact that the APIs are now already checked-in and exposed from the refs, and probably more things I haven't listed. I simply want to make sure this doesn't fall through the cracks, which based on how the issue reads right now, I can totally see it doing.

[joperezr]

I believe that some of the checklist status is not actually up-to-date, but you are right in the fact that this was falling through the cracks. I will sync with the Json Crew to make sure we find an owner for this and complete the work.

[ericstj]

this was falling through the cracks.

The work is in the roadmap for JSON, but at the moment it's not prioritized. The API was reviewed but UX studies pointed towards a change in direction. The right thing to do at the moment is to remove this API so that it doesn't become a hard take-back in later previews. We need to iterate on the design more before we release it.

Either @jozkee or @layomia can you remove the API from the ref as well as whatever supporting source/tests we have? We can keep the source in a branch where we can further work out the design issues.

I've removed @jozkee as the assignee as I don't think he's actively working on this yet.

[mrange]

The API was reviewed but UX studies pointed towards a change in direction.

What does this mean? Writable DOM not of interest to developers? I am certainly able to work around it but I think more than me would find use for a writable DOM.

[ericstj]

What does this mean? Writable DOM not of interest to developers

It is still of interest. The UX study told us that the API we've designed so far was confusing to users. We wanted to iterate on that more before we committed to shipping it stable. Details are outlined above: #30436 (comment)

[layomia]

From @BreyerW in #39610 (comment):

EDIT i scanned the diff in your link and i think i get it how you want to unify but im concerned this way is going to affect negatively current performance with JsonDocument since theres a lot of type checks and type casts. Also theres problem with JsonElement remaining struct (bigger problem imo from usability standpoint)

The benefit of class is that you can effectively 'rip apart' serialized object property by property and the root object will still get updated when single property get updated. Eg databinding scenario where each property is isolated and updated separately. With struct im not sure if that will work nicely (maybe since they are backed by class anyway)

[John0King]

same request here. JSON itself it a format to representation data， and data does not has to be statically structured. the important part is data, not the string. today it looks like that JsonDocument is a string reader and Span<char> in behind (that is called no allocate ), but I think this kind of optimize may only speed up some demo/benchmark program , because in reall application we frequently access and change the "data" , and access to string or other type will be allocate , and string = string is faster than string = read span to string . and I think today that things make json serialize/deserialize slow is because today .net use many Attributes , and access it we need Reflect , and Reflect is slow (and the naming strategy cause we need change the name twice).

I wonder the performace between Newtonsoft.Json and System.Text.Json when only use JObject

[husains]

Just wanted to mention that having the ability to deserialize JSON, make in-memory modifications to the DOM, and then serialize the resulting JSON (Either for persistence or for inclusion as an API response) would be super helpful as an E2E capability in the platform. Looking forward to seeing this ask get prioritized and delivered for .NET 6

[steveharter]

Closing as #29690 is tracking the writable DOM + support for C# "dynamic"

[ericstj]

Update here to make sure that folks noticed. The JSON DOM feature shipped in preview 4 and is available to try. Please let us know how it works for your scenarios. Thanks!
https://devblogs.microsoft.com/dotnet/announcing-net-6-preview-4/#system-text-json-writable-dom-feature