Convert Razor tools from NewtonSoft.Json to System.Text.Json#53345
Merged
Conversation
github-actions
Bot
added
the
Area-AspNetCore
Contributor
|
Thanks for your PR, @@davidwengier. |
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the Razor SDK tool’s TagHelper manifest JSON serialization/deserialization from Newtonsoft.Json to System.Text.Json to help remove Newtonsoft.Json from the SDK layout (per #53287).
Changes:
- Replace Newtonsoft-based JSON read/write pipeline with System.Text.Json equivalents (custom converter + reader/writer abstractions).
- Remove the Newtonsoft.Json package reference from the Razor tool project.
- Add round-trip serialization tests for TagHelperDescriptor JSON payloads.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.NET.Sdk.Razor.Tool.Tests/TagHelperJsonSerializationTest.cs | Adds comprehensive round-trip tests validating TagHelperDescriptor JSON compatibility. |
| src/RazorSdk/Tool/Microsoft.NET.Sdk.Razor.Tool.csproj | Removes Newtonsoft.Json package dependency from the tool. |
| src/RazorSdk/Tool/Json/TagHelperDescriptorJsonConverter.cs | Introduces shared JsonSerializerOptions for the custom converter. |
| src/RazorSdk/Tool/Json/Strings.cs | Replaces Newtonsoft token types with System.Text.Json token types. |
| src/RazorSdk/Tool/Json/ObjectWriters_TagHelpers.cs | Removes now-unneeded helper method overload(s) after writer changes. |
| src/RazorSdk/Tool/Json/ObjectWriters.cs | Removes now-unneeded RazorExtension JSON writer helpers. |
| src/RazorSdk/Tool/Json/ObjectReaders_TagHelpers.cs | Removes now-unneeded TagHelperDescriptor wrapper reader helper. |
| src/RazorSdk/Tool/Json/ObjectJsonConverter`1.cs | Reimplements converter using System.Text.Json reader/writer APIs. |
| src/RazorSdk/Tool/Json/JsonReaderExtensions.cs | Deletes Newtonsoft.Json reader extension utilities. |
| src/RazorSdk/Tool/Json/JsonDataWriter.cs | Replaces JsonWriter-based writer with Utf8JsonWriter-based implementation and prunes unused APIs. |
| src/RazorSdk/Tool/Json/JsonDataReader.cs | Replaces JsonReader-based reader with JsonElement-based implementation and prunes unused APIs. |
| src/RazorSdk/Tool/GenerateCommand.cs | Switches manifest deserialization to System.Text.Json with shared options. |
| src/RazorSdk/Tool/DiscoverCommand.cs | Switches manifest serialization to System.Text.Json with shared options. |
DustinCampbell
approved these changes
Mar 10, 2026
DustinCampbell
left a comment
DustinCampbell
left a comment
Member
There was a problem hiding this comment.
Looks good to me, though it might be nice to have a test that verifies round-tripping the current output of the discover command.
davidwengier
added a commit
to dotnet/razor
that referenced
this pull request
Mar 11, 2026
Sorry, I promised Todd I wouldn't remove any more big swathes of code, but when [converting the SDK](dotnet/sdk#53345) to use System.Text.Json I discovered that none of the serialization code in this repo is actually used there, and so it was a case of either leave this code to be unused, and not matching the SDK, or convert unused code, or just get rid of it. So I got rid of it. Arguably some of this is bits I missed in the last PR anyway (in that PR I removed ProjectSnapshotHandle from being used, but left the code that knew how to serialize it 🤷♂️)
… the generation of the code behind file
|
Great work! Title and description should have mentioned it was only for Razor imo. |
davidwengier
changed the title
Member
Author
|
Good point! |
5 tasks
davidwengier
added a commit
to dotnet/roslyn
that referenced
this pull request
Apr 28, 2026
Sorry, I promised Todd I wouldn't remove any more big swathes of code, but when [converting the SDK](dotnet/sdk#53345) to use System.Text.Json I discovered that none of the serialization code in this repo is actually used there, and so it was a case of either leave this code to be unused, and not matching the SDK, or convert unused code, or just get rid of it. So I got rid of it. Arguably some of this is bits I missed in the last PR anyway (in that PR I removed ProjectSnapshotHandle from being used, but left the code that knew how to serialize it 🤷♂️)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #53287