-
Notifications
You must be signed in to change notification settings - Fork 371
Add migrating-newtonsoft-to-system-text-json skill #89
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
Changes from 1 commit
04a5081
0a64de0
a9285a1
4a507df
9773e1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,249 @@ | ||||||
| ```skill | ||||||
| --- | ||||||
| name: migrating-newtonsoft-to-system-text-json | ||||||
| description: Migrate from Newtonsoft.Json to System.Text.Json, handling behavioral differences, custom converters, and common breaking changes. Use when converting a project from Newtonsoft.Json (Json.NET) to the built-in System.Text.Json serializer. | ||||||
| --- | ||||||
|
|
||||||
| # Migrating from Newtonsoft.Json to System.Text.Json | ||||||
|
|
||||||
| ## When to Use | ||||||
|
|
||||||
| - Migrating an existing project from Newtonsoft.Json to System.Text.Json | ||||||
| - Removing the Newtonsoft.Json dependency for performance or AOT compatibility | ||||||
| - Fixing serialization differences after switching to System.Text.Json | ||||||
|
|
||||||
| ## When Not to Use | ||||||
|
|
||||||
| - The project requires Newtonsoft.Json features that System.Text.Json cannot support (extremely rare edge cases like `$ref/$id` with deep graphs) | ||||||
| - The user is already using System.Text.Json and just needs help with it | ||||||
| - The user explicitly wants to keep Newtonsoft.Json | ||||||
|
|
||||||
| ## Inputs | ||||||
|
|
||||||
| | Input | Required | Description | | ||||||
| |-------|----------|-------------| | ||||||
| | Code using Newtonsoft.Json | Yes | Models, serialization calls, custom converters | | ||||||
| | .NET version | No | Determines which System.Text.Json features are available | | ||||||
|
|
||||||
| ## Workflow | ||||||
|
|
||||||
| ### Step 1: Understand the critical behavioral differences | ||||||
|
|
||||||
| **System.Text.Json is NOT a drop-in replacement.** These behaviors differ by default: | ||||||
|
|
||||||
| | Behavior | Newtonsoft.Json | System.Text.Json | Impact | | ||||||
| |----------|----------------|-------------------|--------| | ||||||
| | **Property naming** | camelCase by default | **PascalCase by default** | APIs will return different JSON | | ||||||
|
mrsharm marked this conversation as resolved.
Outdated
|
||||||
| | **Missing properties** | Ignored silently | Ignored silently | Same ✓ | | ||||||
| | **Extra JSON properties** | Ignored by default | **Throws by default (.NET 8+)** | Deserialization breaks! | | ||||||
|
mrsharm marked this conversation as resolved.
Outdated
|
||||||
| | **Trailing commas** | Allowed | **Rejected by default** | Parse errors on valid-looking JSON | | ||||||
| | **Comments in JSON** | Allowed | **Rejected by default** | Config files break | | ||||||
| | **Number in string** (`"123"`) | Coerced automatically | **Throws by default** | Deserialization breaks! | | ||||||
| | **Enum serialization** | Numeric by default | Numeric by default | Same ✓, but converter syntax differs | | ||||||
| | **null → non-nullable value type** | Sets to default(T) | **Throws exception** | Breaks on dirty data | | ||||||
|
mrsharm marked this conversation as resolved.
Outdated
|
||||||
| | **Case sensitivity** | Case-insensitive | **Case-sensitive by default** | Property matching breaks | | ||||||
|
||||||
| | **Case sensitivity** | Case-insensitive | **Case-sensitive by default** | Property matching breaks | | |
| | **Case sensitivity** | **Case-sensitive by default** | **Case-sensitive by default** | Same by default; configure explicitly for case-insensitive matching | |
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment "Newtonsoft default" on line 67 is incorrect. Newtonsoft.Json does not use camelCase by default for property naming. It uses the property names as-is (typically PascalCase for C# properties). CamelCase requires explicit configuration via CamelCasePropertyNamesContractResolver.
This comment should be removed or corrected to indicate this is a common convention, not a Newtonsoft default.
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment "Newtonsoft default" on line 68 is misleading. PropertyNameCaseInsensitive is not a Newtonsoft.Json default behavior. Newtonsoft.Json is case-sensitive by default during deserialization, just like System.Text.Json.
This comment should be removed or changed to clarify that this is an optional configuration, not matching a Newtonsoft default.
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pitfall "Forgetting PropertyNameCaseInsensitive = true" implies this is a required configuration to match Newtonsoft.Json behavior, but this is not accurate. Both libraries are case-sensitive by default.
This pitfall should be reworded to clarify that PropertyNameCaseInsensitive is only needed if the previous Newtonsoft.Json configuration explicitly enabled case-insensitive deserialization, not as a default requirement for all migrations.
| | Forgetting `PropertyNameCaseInsensitive = true` | Deserialization silently returns default values for all properties | | |
| | Assuming case-insensitive property matching without configuring it | If your previous Newtonsoft.Json settings enabled case-insensitive property names, set `options.PropertyNameCaseInsensitive = true` in `JsonSerializerOptions`; otherwise both serializers are case-sensitive by default and differing JSON/property casing will deserialize to default values. | |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,61 @@ | ||||||
| scenarios: | ||||||
| - name: "Migrate model with Newtonsoft.Json attributes to System.Text.Json" | ||||||
| prompt: | | ||||||
| I'm migrating our ASP.NET Core 8 project from Newtonsoft.Json to System.Text.Json. Here's a model class that uses Newtonsoft attributes and a custom converter. Convert this to System.Text.Json: | ||||||
|
|
||||||
| ```csharp | ||||||
| using Newtonsoft.Json; | ||||||
| using Newtonsoft.Json.Converters; | ||||||
| using Newtonsoft.Json.Linq; | ||||||
|
|
||||||
| public class Order | ||||||
| { | ||||||
| [JsonProperty("order_id")] | ||||||
| public int Id { get; set; } | ||||||
|
|
||||||
| [JsonProperty(Required = Required.Always)] | ||||||
| public string CustomerName { get; set; } | ||||||
|
|
||||||
| [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] | ||||||
| public string? Notes { get; set; } | ||||||
|
|
||||||
| [JsonConverter(typeof(StringEnumConverter))] | ||||||
| public OrderStatus Status { get; set; } | ||||||
|
|
||||||
| [JsonExtensionData] | ||||||
| public Dictionary<string, JToken>? AdditionalData { get; set; } | ||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| Also show me how to configure the JSON options globally to match Newtonsoft.Json's default behavior. | ||||||
|
||||||
| assertions: | ||||||
| - type: "output_contains" | ||||||
| value: "JsonPropertyName" | ||||||
| - type: "output_matches" | ||||||
| pattern: "(JsonIgnore.*WhenWritingNull|JsonIgnoreCondition)" | ||||||
| - type: "output_matches" | ||||||
| pattern: "(PropertyNameCaseInsensitive|CamelCase|PropertyNamingPolicy)" | ||||||
| - type: "output_matches" | ||||||
| pattern: "(JsonElement|JsonNode)" | ||||||
| rubric: | ||||||
| - "Replaced [JsonProperty(\"order_id\")] with [JsonPropertyName(\"order_id\")]" | ||||||
| - "Replaced NullValueHandling.Ignore with [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]" | ||||||
| - "Replaced [JsonConverter(typeof(StringEnumConverter))] with System.Text.Json equivalent (JsonStringEnumConverter)" | ||||||
| - "Changed [JsonExtensionData] Dictionary value type from JToken to JsonElement (critical difference!)" | ||||||
| - "Configured PropertyNameCaseInsensitive = true to match Newtonsoft default case-insensitive behavior" | ||||||
|
||||||
| - "Configured PropertyNameCaseInsensitive = true to match Newtonsoft default case-insensitive behavior" | |
| - "Discussed PropertyNameCaseInsensitive and how it compares to Newtonsoft's default case-sensitive behavior" |
Copilot
AI
Feb 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rubric expects warnings about "default casing" as a behavioral difference, but since both libraries use the same default casing (property names as-is, typically PascalCase), this warning would be misleading.
The rubric should focus on actual behavioral differences such as strict JSON parsing, numbers-as-strings handling, and trailing commas/comments.
| - "Warned about behavioral differences (default casing, strict parsing)" | |
| - "Warned about behavioral differences (strict parsing, numbers-as-strings, trailing commas/comments)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The skill metadata format is incorrect. Skills in this repository use standard YAML frontmatter delimited by
---(triple dashes), not code fence blocks with```skill.The skill-validator expects frontmatter in this format:
The current
```skillformat will not be parsed correctly by the discovery system (see eng/skill-validator/src/discovery.ts lines 11-16 which specifically looks for the---delimited frontmatter pattern).