Skip to content
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

Developers using System.Text.Json have their top requests addressed so that they can use STJ in more scenarios #45190

Closed
4 of 7 tasks
terrajobst opened this issue Nov 25, 2020 · 14 comments
Labels
area-System.Text.Json Cost:L Work that requires one engineer up to 4 weeks Priority:1 Work that is critical for the release, but we could probably ship without tracking This issue is tracking the completion of other related issues. User Story A single user-facing feature. Can be grouped under an epic.
Projects
Milestone

Comments

@terrajobst
Copy link
Member

terrajobst commented Nov 25, 2020

Work

@terrajobst terrajobst added area-System.Text.Json Cost:L Work that requires one engineer up to 4 weeks labels Nov 25, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Nov 25, 2020
@terrajobst terrajobst removed the untriaged New issue has not been triaged by the area owner label Nov 25, 2020
@terrajobst terrajobst added the User Story A single user-facing feature. Can be grouped under an epic. label Nov 25, 2020
@terrajobst terrajobst added this to Proposed in .NET 6.0 Nov 25, 2020
@marek-safar marek-safar added tracking This issue is tracking the completion of other related issues. and removed User Story A single user-facing feature. Can be grouped under an epic. labels Nov 27, 2020
@MhAllan
Copy link

MhAllan commented Dec 1, 2020

JsonSerializer TimeSpan support - is very important, everyone run back to Newtonsoft because of this.

@Rwing
Copy link

Rwing commented Dec 9, 2020

The pace is too slow, everyone need this features now. maybe put it in 5.0.2 or 5.1 is better.

@Romfos
Copy link

Romfos commented Dec 23, 2020

Allow support for non-public members +++++

@ishepherd
Copy link

Can any of these be plugged in naturally by a 3rd party if they had the right extensibility support?

A 3rd party library can evolve so much easier than .NET

@layomia layomia changed the title Miscellaneous improvements in System.Text.Json User Story: Developers using System.Text.Json want miscellaneous features so that they can use STJ in more scenarios Jan 21, 2021
@layomia layomia added the User Story A single user-facing feature. Can be grouped under an epic. label Jan 21, 2021
@danmoseley danmoseley changed the title User Story: Developers using System.Text.Json want miscellaneous features so that they can use STJ in more scenarios Developers using System.Text.Json have their top requests addressed so that they can use STJ in more scenarios Jan 21, 2021
@ericstj ericstj added the Priority:1 Work that is critical for the release, but we could probably ship without label Jan 22, 2021
@Trolldemorted
Copy link

Failing null deserialization on non-nullable references types (#1256) would be awesome. Right now we have to check the entire children tree whether there was a null value, which is very annoying.

@hez2010
Copy link
Contributor

hez2010 commented Feb 24, 2021

Please allow to leave "null" when serialize unsupported types.
Currently if a type is not owned by myself and it contains a ref struct property (eg. Span<T>), I have no way to serialize it to JSON because System.Text.Json will always throw a NotSupportedException.
Newtonsoft.Json can just skip the unsupported properties and leave them as null.

@dan655t
Copy link

dan655t commented Mar 8, 2021

Would also like to see full emoji support (#42847). Without it we're unable to use System.Text.Json for user generated content.

Edit: this can be disregarded. I have closed (#42847).

@BreyerW
Copy link

BreyerW commented Apr 21, 2021

@layomia i see my #42163 got removed but i dont see it being moved elsewhere. It got triaged out or deleted by accident? Just curious in worst case i will fork S.T.J when this lib gets all other major features that are missing from json.net

@layomia
Copy link
Contributor

layomia commented Apr 21, 2021

@BreyerW I was updating the related issue #40099 as done for 6.0 and noticed that #42163 is marked for "Future". I recall discussing it with @Jozkee earlier - it strikes me as a niche and design-heavy problem that we might not get to in 6.0. It would be good to see more feedback from other customers to prioritize the work for the current release. My intention when adding it to this list was to remember assess to its feasibility for 6.0. At this point it seems unlikely, given the other JSON work we need to do in this release (https://themesof.net/?q=json%20is:open%20kinds:teu).

Can we continue the discussion over at #42163? The feature to deserialize into an existing object (#29538) was mentioned as a potential workaround and is more likely to make it to 6.0 given it's relative technical simplicity, precedent in Newtonsoft.Json, and demand from multiple customers.

@MuleaneEve
Copy link

MuleaneEve commented May 27, 2021

JsonSerializer TimeSpan support - is very important, everyone run back to Newtonsoft because of this.

Please, also add support for the new DateOnly and TimeOnly structs.
Edit: Otherwise, they will be avoided in any scenario that also uses STJ.

@layomia
Copy link
Contributor

layomia commented Jun 1, 2021

I updated the work items:

Added

Moved to "More extensible object and collection converters" #36785

JsonSerializer will not look for these attributes itself, but can provide APIs for users to do so and populate relevant JsonTypeInfo, JsonPropertyInfo members which will honor those options.

Moved to future

These are good features but can be addressed later given other work we need to do for this release.

@daiplusplus
Copy link

daiplusplus commented Jul 22, 2021

I know System.Text.Json won't invoke private members by default, but I'm surprised and disappointed that we cannot explicitly denote private constructors with [JsonConstructor].

Ideally I'd like to reuse the same DTO types in Newtonsoft.Json and System.Text.Json contexts (with suitable using JsonPropertyAttribute = System.Text.Json.Serialization.JsonPropertyNameAttribute in #if USE_STJ at the top of the .cs file), however my projects tend to use primary-constructors which STJ doesn't support, so I can't use the above trick just yet.

  • Allow [JsonConstructor] with private constructors, even when a public constructor is available.
    • This is essential for cases where a type has different constructors for different states, with per-state validation logic (for an example, see my class SqlDbTypeRef in the expanding region below). Exposing a public constructor that has parameters for all properties would allow consumers to unwittingly circumvent the validation rules - and requiring us to reimplementing the validation logic in this new constructor is not endearing me to use STJ over NJ.
  • Allow [JsonPropertyName] on constructor parameters and use JsonPropertyNameAttribute.Name to map them to C# properties' JsonPropertyNameAttribute.Name instead of their C# property names.
    • This make it easier to write DTOs that are more resilient to unintentional symbol renames when refactoring - especially as Visual Studio's C# Refactor-Rename tool does not currently rename constructor parameters that are assigned to POCO properties.
    • Allowing explicit JSON property names on ctor parameters also allows us to tweak serialization vs. deserialization logic without needing too much effort.
    • A use-case for this is when you want the same DTO to intentionally ignore or drop certain properties when deserializing but still serialize them and you're using code-gen or T4 to generate DTOs and having to use custom logic in the ctor is just too much effort.
  • Allow partial [JsonConstructor] parameter-to-property mapping.
    • This is useful when the private [JsonConstructor] is also used by a public static factory method and additional details are routed through there but would not come from JSON - so the constructor has more parameters than JSON properties.

For example, I have a class that represents a SqlDbType with length, precision, and scale parameters - this is what I currently have working with Newtonsoft.Json:


Click to expand!
public partial class SqlDbTypeRef
{
	/// <summary>For <see cref="System.Data.SqlDbType"/> types without parameters.</summary>
	/// <exception cref="ArgumentException">When the <paramref name="dbType"/> value used requires length, precision, or scale parameter values.</exception>
	public SqlDbTypeRef( SqlDbType dbType )
	{
		SqlDbTypeParameters p = SqlDbTypeExtensions.GetParameters( dbType );
		if( p == SqlDbTypeParameters.None )
		{
			Int16? length = null;
			if( SqlDbTypeExtensions.HasImplicitLength( dbType, out Int16 implicitLength ) ) length = implicitLength;

			this.SqlDbType = dbType;
			this.Length    = length;
			this.Precision = null;
			this.Scale     = null;
		}
		else
		{
			throw new ArgumentException( message: "SqlDbType.{0} is parameterized: {1}".Fmt( dbType, p ), paramName: nameof(dbType) );
		}
	}
		
	/// <summary>For <see cref="SqlDbType.Binary"/>, <see cref="SqlDbType.VarBinary"/>, <see cref="SqlDbType.Char"/>, <see cref="SqlDbType.NChar"/>, <see cref="SqlDbType.NVarChar"/>, <see cref="SqlDbType.VarChar"/>.</summary>
	public SqlDbTypeRef( SqlDbType dbType, Int16 length )
	{
		SqlDbTypeParameters p = SqlDbTypeExtensions.GetParameters( dbType );
		if( p == SqlDbTypeParameters.Length )
		{
			this.SqlDbType = dbType;
			this.Length    = length;
			this.Precision = null;
			this.Scale     = null;
		}
		else
		{
			throw new ArgumentException( message: "SqlDbType.{0} requires {1}".Fmt( dbType, p ), paramName: nameof(dbType) );
		}
	}

	/// <summary>For <see cref="SqlDbType.DateTime2"/>, <see cref="SqlDbType.DateTimeOffset"/>, <see cref="SqlDbType.Float"/>.</summary>
	public SqlDbTypeRef( SqlDbType dbType, Byte precision )
	{
		SqlDbTypeParameters p = SqlDbTypeExtensions.GetParameters( dbType );
		if( p == SqlDbTypeParameters.Precision )
		{
			this.SqlDbType = dbType;
			this.Length    = null;
			this.Precision = precision;
			this.Scale     = null;
		}
		else
		{
			throw new ArgumentException( message: "SqlDbType.{0} requires {1}".Fmt( dbType, p ), paramName: nameof(dbType) );
		}
	}

	/// <summary>For <see cref="SqlDbType.Decimal"/>.</summary>
	public SqlDbTypeRef( SqlDbType dbType, Byte precision, Byte scale )
	{
		SqlDbTypeParameters p = SqlDbTypeExtensions.GetParameters( dbType );
		if( p == SqlDbTypeParameters.PrecisionAndScale )
		{
			this.SqlDbType = dbType;
			this.Length    = null;
			this.Precision = precision;
			this.Scale     = scale;
		}
		else
		{
			throw new ArgumentException( message: "SqlDbType.{0} requires {1}".Fmt( dbType, p ), paramName: nameof(dbType) );
		}
	}

	[JsonConstructor]
	private SqlDbTypeRef(
		[JsonProperty( "type"   )] SqlDbType dbType,
		[JsonProperty( "length" )] Int16?    length,
		[JsonProperty( "precision" )] Byte?     precision,
		[JsonProperty( "scale"  )] Byte?     scale
	)
	{
		this.SqlDbType = dbType;
		this.Length    = length;
		this.Precision = precision;
		this.Scale     = scale;
	}

	[JsonProperty( "type" )]
	public SqlDbType SqlDbType { get; }

	[JsonProperty( "length", NullValueHandling = NullValueHandling.Ignore )]
	public Int16?    Length    { get; }
		
	[JsonProperty( "precision", NullValueHandling = NullValueHandling.Ignore )]
	public Byte?     Precision { get; }
		
	[JsonProperty( "scale", NullValueHandling = NullValueHandling.Ignore )]
	public Byte?     Scale     { get; }
}

@layomia
Copy link
Contributor

layomia commented Jul 23, 2021

@Jehoel

Allow [JsonConstructor] with private constructors, even when a public constructor is available.

This is tracked by #31511.

Allow [JsonPropertyName] on constructor parameters and use JsonPropertyNameAttribute.Name to map them to C# properties' JsonPropertyNameAttribute.Name instead of their C# property names.

Kindly open a new issue to track this feature request.

Allow partial [JsonConstructor] parameter-to-property mapping.

This is generally tracked by #44428. Please add a note on that issue describing the sort of functionality enhancements you'd like to see.

@layomia
Copy link
Contributor

layomia commented Jul 23, 2021

Triage - we can consider this work done and I'll close this issue. We've addressed what we could in the .NET 6.0 timeframe, and can consider the other tasks in .NET 7.0 planning.

@layomia layomia closed this as completed Jul 23, 2021
.NET 6.0 automation moved this from Proposed to Completed Jul 23, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json Cost:L Work that requires one engineer up to 4 weeks Priority:1 Work that is critical for the release, but we could probably ship without tracking This issue is tracking the completion of other related issues. User Story A single user-facing feature. Can be grouped under an epic.
Projects
.NET 6.0
  
Completed
Development

No branches or pull requests