Add table of differences to Migrate from Newtonsoft.Json doc #16704

tdykstra · 2020-01-15T23:29:32Z

Fixes #16309

Internal review URL

ahsonkhan

Some thoughts about table ordering/grouping.

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

ahsonkhan · 2020-01-16T00:58:38Z

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

+## Table of differences between Newtonsoft.Json and System.Text.Json
+
+| `Newtonsoft.Json` feature                           | `System.Text.Json` equivalent |
+|-----------------------------------------------------|-------------------------------|


Should we sort these grouping all the Supported, Not Supported and other categories in the System.Text.Json equivalent column? Knowing all the things that are in the "not supported" category might be helpful.

Alternatively (or in addition to), we do something like annotate the text with images in front of the text):
green checkmark - supported (but maybe differently)
yellow checkmark - supported/possible (workaround
red x - not supported

I'll group them by supported, workaround with sample, workaround no sample, not supported. I'm looking into getting the color-coded checkmark and X icons to add.

ahsonkhan · 2020-01-16T01:03:03Z

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

+| `DefaultValueHandling` setting on `[JsonProperty]` attribute | [Requires a custom converter, sample provided](#conditionally-ignore-a-property)  |
+| Callbacks                                           | [Requires a custom converter, sample provided](#callbacks) |
+| Support for a broad range of types                  | [Some types require custom converters](#types-without-built-in-support) |
+| Support for public and non-public fields            | [Requires a custom converter](#public-and-non-public-fields) |


Other type of ordering we could consider is what's more common. In that case, items like these should be more on the top where as some of the other/less used features at the bottom. What were you thinking in terms of sort order?

Also, I thought things like these are in the "not supported" column (they are in the Scenarios that JsonSerializer currently doesn't support heading), no?

The overall order now is by groups: supported, workaround with sample, workaround without sample, no workaround practical. Within each grouping ideally we'd go from more common to less common, but it's not always easy to differentiate on that basis. I'm open to suggestions on changing the order.

The Scenarios that JsonSerializer currently doesn't support section originally was two sections that got conflated -- workaround-no-sample and workaround-not-practical. We could split it up again, which would more closely match the groupings in the table.

The overall order now is by groups: supported, workaround with sample, workaround without sample, no workaround practical. Within each grouping ideally we'd go from more common to less common, but it's not always easy to differentiate on that basis.

Perfect.

I'm open to suggestions on changing the order.

Provided some thoughts. Thanks.

We could split it up again, which would more closely match the groupings in the table.

This doc will likely keep evolving, especially until we ship 5.0. So, let's leave that change out for now.

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

pranavkm · 2020-01-16T19:26:22Z

This looks super nice! ❤️

gewarren

Looks good.

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

ahsonkhan

Other than some ordering suggestions, looks great!

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

ahsonkhan · 2020-01-16T21:20:15Z

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

+| `JsonTextReader`                                      | ![check mark icon](../../../images/core/check-mark.png) [Utf8JsonReader](#utf8jsonreader-compared-to-jsontextreader) |
+| `JsonTextWriter`                                      | ![check mark icon](../../../images/core/check-mark.png) [Utf8JsonWriter](#utf8jsonwriter-compared-to-jsontextwriter) |
+| Support for a broad range of types                    | ![warning icon](../../../images/core/yellow-warning.png) [Some types require custom converters](#types-without-built-in-support) |
+| Deserialize inferred type to `object` properties      | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround, sample](#deserialization-of-object-properties) |


Here's the ordering for this section (most of it looks good):
Support for a broad range of types
Deserialize inferred type to object properties
Deserialize strings as numbers
Deserialize Dictionary with non-string key
Polymorphic serialization
Polymorphic deserialization
Deserialize JSON null literal to non-nullable types
Deserialize to immutable classes and structs
[JsonConstructor] attribute
Required setting on [JsonProperty] attribute
NullValueHandling setting on [JsonProperty] attribute
DefaultValueHandling setting on [JsonProperty] attribute
DefaultValueHandling global setting
DefaultContractResolver to exclude properties
DateTimeZoneHandling, DateFormatString settings
Callbacks

ahsonkhan · 2020-01-16T21:20:55Z

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

+| `JsonConvert.PopulateObject` method                   | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround](#populate-existing-objects) |
+| `ObjectCreationHandling` global setting               | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround](#reuse-rather-than-replace-properties) |
+| Add to collections without setters                    | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround](#add-to-collections-without-setters) |
+| `PreserveReferencesHandling` global setting           | ![red x icon](../../../images/core/x.png) [Not supported](#preserve-object-references-and-handle-loops) |


the ordering of the Not supported section looks good to me.

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

ahsonkhan · 2020-01-16T21:33:51Z

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

+| Deserialize `Dictionary` with non-string key          | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround, sample](#dictionary-with-non-string-key) |
+| Polymorphic serialization                             | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround, sample](#polymorphic-serialization) |
+| Polymorphic deserialization                           | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround, sample](#polymorphic-deserialization) |
+| `Required` setting on [JsonProperty] attribute        | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround, sample](#required-properties) |


Consistency with others:

Suggested change

| `Required` setting on [JsonProperty] attribute | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround, sample](#required-properties) |

| `Required` setting on `[JsonProperty]` attribute | ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround, sample](#required-properties) |

Also, consider adding a sentence referencing the attribute there - In Newtonsoft.Json you would specifiy that a property is required by setting Required on the the [JsonProperty] attribute.

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

ahsonkhan · 2020-01-17T02:23:23Z

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

-During serialization, `Newtonsoft.Json` is relatively permissive about letting characters through without escaping them. That is, it doesn't replace them with `\uxxxx` where `xxxx` is the character's code point. Where it does escape them, it does so by emitting a `\` before the character (for example, `"` becomes `\"`). <xref:System.Text.Json> escapes more characters by default to provide defense-in-depth protections against cross-site scripting (XSS) or information-disclosure attacks and does so by using the six-character sequence. `System.Text.Json` escapes all non-ASCII characters by default, so you don't need to do anything if you're using `StringEscapeHandling.EscapeNonAscii` in `Newtonsoft.Json`. `System.Text.Json` also escapes HTML-sensitive characters, by default. For information about how to override the default `System.Text.Json` behavior, see [Customize character encoding](system-text-json-how-to.md#customize-character-encoding).
+Custom converters can be implemented for types that don't have built-in support.

 ### Deserialization of object properties


I am fine with the table ordering for this section, but the doc section order doesn't match for this. Move this section down below the polymorphic deserialization section.

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

ahsonkhan

One leftover feedback. Otherwise, look's great!

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

…onsoft-how-to.md Co-Authored-By: Ahson Khan <ahkha@microsoft.com>

ahsonkhan · 2020-01-17T04:38:28Z

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md

+|-------------------------------------------------------|-----------------------------|
+| Case-insensitive deserialization by default           | ✔️ [PropertyNameCaseInsensitive global setting](#case-insensitive-deserialization) |
+| Camel-case property names                             | ✔️ [PropertyNamingPolicy global setting](system-text-json-how-to.md#use-camel-case-for-all-json-property-names)
+ |


This shouldn't be on a new line. It shows up as an empty row:

BillWagner

This really looks great @tdykstra

You can when ready.

add table of differences

093cd48

dotnet-bot added this to the January 2020 milestone Jan 15, 2020

dotnet-bot added the 📚 Area - .NET Guide label Jan 15, 2020

tdykstra requested review from BillWagner and ahsonkhan January 15, 2020 23:36

ahsonkhan reviewed Jan 16, 2020

View reviewed changes

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md Outdated Show resolved Hide resolved

address feedback

4506487

tdykstra requested a review from pranavkm January 16, 2020 19:06

fix copy-paste error

f262794

tdykstra added 2 commits January 16, 2020 11:53

icons

97b6936

fix icon

b0888ec

gewarren reviewed Jan 16, 2020

View reviewed changes

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md Outdated Show resolved Hide resolved

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md Outdated Show resolved Hide resolved

fix find-replace error

698ff56

ahsonkhan reviewed Jan 16, 2020

View reviewed changes

tdykstra added 4 commits January 16, 2020 15:11

use emojis instead

11a3b05

move jsondocument etc out of table

6429c51

remove image files

06d7262

feedback

54ad9b2

ahsonkhan reviewed Jan 17, 2020

View reviewed changes

address feedback

a8f0868

ahsonkhan approved these changes Jan 17, 2020

View reviewed changes

docs/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to.md Outdated Show resolved Hide resolved

Update docs/standard/serialization/system-text-json-migrate-from-newt…

62246e3

…onsoft-how-to.md Co-Authored-By: Ahson Khan <ahkha@microsoft.com>

ahsonkhan reviewed Jan 17, 2020

View reviewed changes

remove extraneous newline

526a932

BillWagner approved these changes Jan 17, 2020

View reviewed changes

tdykstra merged commit 0430d46 into dotnet:master Jan 17, 2020

tdykstra deleted the stjtable branch January 17, 2020 15:31

	\| `Required` setting on [JsonProperty] attribute \| ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround, sample](#required-properties) \|
	\| `Required` setting on `[JsonProperty]` attribute \| ![warning icon](../../../images/core/yellow-warning.png) [Not supported, workaround, sample](#required-properties) \|

Add table of differences to Migrate from Newtonsoft.Json doc #16704

Add table of differences to Migrate from Newtonsoft.Json doc #16704

Uh oh!

Conversation

tdykstra commented Jan 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahsonkhan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pranavkm commented Jan 16, 2020

Uh oh!

gewarren left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ahsonkhan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ahsonkhan Jan 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ahsonkhan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BillWagner left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

tdykstra commented Jan 15, 2020 •

edited

Loading

ahsonkhan Jan 17, 2020 •

edited

Loading