Skip to content

Change Json.NET to System.Text.Json #3758

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Mar 25, 2020
Merged

Change Json.NET to System.Text.Json #3758

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1bcc5a5
Change Json.NET to System.Text.Json
tdykstra Jan 10, 2020
4630528
move serializer advice to remarks
tdykstra Mar 25, 2020
01bfb71
fix indentation
tdykstra Mar 25, 2020
eab8044
fix typo
tdykstra Mar 25, 2020
5a47156
fix link
tdykstra Mar 25, 2020
5768f70
4.6.1 --> 4.7.2
tdykstra Mar 25, 2020
5a1d082
include use s.t.j/newtonsoft advice in summary
tdykstra Mar 25, 2020
7324933
fix typo
tdykstra Mar 25, 2020
4350324
Update xml/System.Web.Script.Serialization/JavaScriptSerializer.xml
tdykstra Mar 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion xml/System.Web.Script.Serialization/JavaScriptSerializer.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<Interfaces />
<Docs>
<summary>
<see href="https://www.newtonsoft.com/json">Json.NET</see> should be used for serialization and deserialization. Provides serialization and deserialization functionality for AJAX-enabled applications.</summary>
The APIs in the <see cref="N:System.Text.Json" /> namespace should be used for serialization and deserialization. Provides serialization and deserialization functionality for AJAX-enabled applications.</summary>
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this statement true ONLY for .NET Framework 4.6.1 or later?

Copy link
Member

Choose a reason for hiding this comment

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

Use Json.NET instead of this class

Yes, I recall switching from JavaScriptSerializer to JSON.NET years ago for performance reasons. At one point, there was messaging from Microsoft that completely discouraged the use of JavaScriptSerializer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ahsonkhan In this doc should we say use Newtonsoft.json for earlier than 4.6.1 and System.Text.Json for 4.6.1 and later?

Copy link
Contributor

Choose a reason for hiding this comment

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

@layomia @jozkee @steveharter can you please help answer this question?

Copy link
Contributor

Choose a reason for hiding this comment

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

@steveharter @layomia @jozkee ping. What is the correct sentence to use here?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, System.Text.Json targets netstandard2.0 which makes it compatible only with .NET Framework 4.6.1 or later.

should we say use Newtonsoft.json for earlier than 4.6.1 and System.Text.Json for 4.6.1 and later

That seems to be the most accurate. Why is that advice on the summary and not as a warning in the JavaScriptSerializer page?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is that advice on the summary and not as a warning in the JavaScriptSerializer page?

No way to know why it's there or who put it there -- it's already there in the earliest commit in the repo.

I agree it's out of place where it is, I'll move it to Remarks.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll move it to Remarks.

If we only have it in remarks, it won't show up in IntelliSense for folks using IDEs like VS. We may want to keep the note in the summary as well as the remarks.

<remarks>
<format type="text/markdown"><![CDATA[

Expand Down