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

[API Proposal]: Expose a JsonSerializerOptions.Default property #61093

Closed
Tracked by #63762
eiriktsarpalis opened this issue Nov 2, 2021 · 4 comments · Fixed by #61434
Closed
Tracked by #63762

[API Proposal]: Expose a JsonSerializerOptions.Default property #61093

eiriktsarpalis opened this issue Nov 2, 2021 · 4 comments · Fixed by #61434
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json Cost:S Work that requires one engineer up to 1 week Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@eiriktsarpalis
Copy link
Member

Background and motivation

JsonSerializerOptions currently exposes an internal s_defaultOptions instance for use by System.Text.Json. This is useful since JsonSerializerOptions holds the reflection-based serialization metadata cache, as such creating a default new JsonSerializerOptions() instance can be very expensive since it forces the regeneration of that cache.

I propose we make this singleton a public property so that users can read default configuration cheaply (e.g. the default JsonConverter for a requested type).

API Proposal

namespace System.Text.Json
{
    public partial class JsonSerializerOptions
    {
        public static JsonSerializerOptions Default { get; }
    }
}

API Usage

Here's an (artificial) example of how the property can be used to cheaply recover the default converter used by STJ for integers:

public class MyCustomConverter : JsonConverter<int>
{
    private readonly static JsonConverter<int> s_defaultConverter = (JsonConverter<int>)JsonSerializerOptions.Default.GetConverter(typeof(int));

    // custom serialization logic
    public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options)
    {
        return writer.WriteStringValue(value.ToString());
    }

    // fall back to default deserialization logic
    public override int Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return s_defaultConverter.Read(ref reader, typeToConvert, options);
    }
}

Alternative Designs

No response

Risks

No response

@eiriktsarpalis eiriktsarpalis added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json labels Nov 2, 2021
@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Nov 2, 2021
@eiriktsarpalis eiriktsarpalis self-assigned this Nov 2, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Nov 2, 2021
@ghost
Copy link

ghost commented Nov 2, 2021

Tagging subscribers to this area: @dotnet/area-system-text-json
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

JsonSerializerOptions currently exposes an internal s_defaultOptions instance for use by System.Text.Json. This is useful since JsonSerializerOptions holds the reflection-based serialization metadata cache, as such creating a default new JsonSerializerOptions() instance can be very expensive since it forces the regeneration of that cache.

I propose we make this singleton a public property so that users can read default configuration cheaply (e.g. the default JsonConverter for a requested type).

API Proposal

namespace System.Text.Json
{
    public partial class JsonSerializerOptions
    {
        public static JsonSerializerOptions Default { get; }
    }
}

API Usage

Here's an (artificial) example of how the property can be used to cheaply recover the default converter used by STJ for integers:

public class MyCustomConverter : JsonConverter<int>
{
    private readonly static JsonConverter<int> s_defaultConverter = (JsonConverter<int>)JsonSerializerOptions.Default.GetConverter(typeof(int));

    // custom serialization logic
    public override void Write(Utf8JsonWriter writer, int value, JsonSerializerOptions options)
    {
        return writer.WriteStringValue(value.ToString());
    }

    // fall back to default deserialization logic
    public override int Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return s_defaultConverter.Read(ref reader, typeToConvert, options);
    }
}

Alternative Designs

No response

Risks

No response

Author: eiriktsarpalis
Assignees: eiriktsarpalis
Labels:

api-suggestion, area-System.Text.Json

Milestone: 7.0.0

@eiriktsarpalis eiriktsarpalis added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Nov 2, 2021
@TonyValenti
Copy link

Would the defaults be mutable? My recommendation is "No".

@eiriktsarpalis
Copy link
Member Author

It would not be. Moreover the implementation of this property would need to make sure that the instance is locked for modification before being returned.

@terrajobst
Copy link
Member

terrajobst commented Nov 2, 2021

Video

  • We need to make sure the instance is marked as "you can't touch this anymore"
namespace System.Text.Json
{
    public partial class JsonSerializerOptions
    {
        public static JsonSerializerOptions Default { get; }
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 2, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 10, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 15, 2021
@eiriktsarpalis eiriktsarpalis added User Story A single user-facing feature. Can be grouped under an epic. Team:Libraries Cost:S Work that requires one engineer up to 1 week labels Jan 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json Cost:S Work that requires one engineer up to 1 week Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants