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]: System.Text.Json serialization callback context #59892

Closed
IdkGoodName opened this issue Oct 2, 2021 · 5 comments
Closed

[API Proposal]: System.Text.Json serialization callback context #59892

IdkGoodName opened this issue Oct 2, 2021 · 5 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Text.Json
Milestone

Comments

@IdkGoodName
Copy link

Background and motivation

Newtonsoft.Json allows you to pass context to its JSON serializer via serializer settings that can later be retrieved in the callbacks. This works well enough for objects that need a specific property (like parent clients, status, etc.) to be set as passed value and it works at any depth. Unfortunately, there isn't a way to do that with System.Text.Json apart from using custom converters, that can't have serializer settings to be passed down (and you can't pass the property value to child types that require same property set if settings aren't passed). This can make it harder to move certain projects over to System.Text.Json and if not make it worse in some instances.

API Proposal

namespace System.Text.Json.Serialization
{
    public interface IJsonOnDeserialized<T>
    {
        void OnDeserialized(SomeContextType<T> context);
    }
}

or with the breaking change:

namespace System.Text.Json.Serialization
{
    public interface IJsonOnDeserialized
    {
        void OnDeserialized(StreamingContext context);
    }
}

API Usage

JsonSerializerOptions options = new JsonSerializerOptions
{
    // ...
    Context = new SomeContextType<int>(10),
    // ... 
};
public class SomeObject : IJsonOnDeserialized<int>
{
    public int Status { get; set; }
 
    public void IJsonOnDeserialized.OnDeserialized(SomeContextType<int> context) => Status = context.Value;
    
    // ... Now we can set up methods or anything else with the received context ...
}

Risks

This could lead to breaking changes if callback interfaces wouldn't use generic types.

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

ghost commented Oct 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

Newtonsoft.Json allows you to pass context to its JSON serializer via serializer settings that can later be retrieved in the callbacks. This works well enough for objects that need a specific property (like parent clients, status, etc.) to be set as passed value and it works at any depth. Unfortunately, there isn't a way to do that with System.Text.Json apart from using custom converters, that can't have serializer settings to be passed down (and you can't pass the property value to child types that require same property set if settings aren't passed). This can make it harder to move certain projects over to System.Text.Json and if not make it worse in some instances.

API Proposal

namespace System.Text.Json.Serialization
{
    public interface IJsonOnDeserialized<T>
    {
        void OnDeserialized(SomeContextType<T> context);
    }
}

or with the breaking change:

namespace System.Text.Json.Serialization
{
    public interface IJsonOnDeserialized
    {
        void OnDeserialized(StreamingContext context);
    }
}

API Usage

JsonSerializerOptions options = new JsonSerializerOptions
{
    // ...
    Context = new SomeContextType<int>(10),
    // ... 
};
public class SomeObject : IJsonOnDeserialized<int>
{
    public int Status { get; set; }
 
    public void IJsonOnDeserialized.OnDeserialized(SomeContextType<int> context) => Status = context.Value;
    
    // ... Now we can set up methods or anything else with the received context ...
}

Risks

This could lead to breaking changes if callback interfaces wouldn't use generic types.

Author: IdkGoodName
Assignees: -
Labels:

api-suggestion, area-System.Text.Json, untriaged

Milestone: -

@eiriktsarpalis
Copy link
Member

This was discussed in the serialization callback API proposal, see #54528 (comment). I understand passing contexts was cut since it would be difficult to support in the current iteration of source generation.

cc @steveharter

@eiriktsarpalis eiriktsarpalis added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner labels Oct 11, 2021
@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Oct 11, 2021
@ghost
Copy link

ghost commented Oct 11, 2021

This issue has been marked with the api-needs-work label. This may suggest that the proposal requires further refinement before it can be considered for API review. Please refer to our API review guidelines for a detailed description of the process.

When ready to submit an amended proposal, please ensure that the original post in this issue has been updated, following the API proposal template and examples as provided in the guidelines.

@eiriktsarpalis
Copy link
Member

This would likely need to be considered in combination with #36785.

@eiriktsarpalis eiriktsarpalis removed the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 16, 2021
@eiriktsarpalis
Copy link
Member

JsonSerializer doesn't support passing user state for the moment. This is something we might consider in #71718 and will be implemented in tandem with #63795. Closing as duplicate of #71718.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Text.Json
Projects
None yet
Development

No branches or pull requests

2 participants