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

Add a JsonObject.TryAdd method. #110244

Open
eiriktsarpalis opened this issue Nov 28, 2024 · 2 comments
Open

Add a JsonObject.TryAdd method. #110244

eiriktsarpalis opened this issue Nov 28, 2024 · 2 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json
Milestone

Comments

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Nov 28, 2024

This workaround requires dual lookup if the key is missing, which suggests to me that a JsonObject.TryAddmethod similar to the one in #107947 might be necessary.

Originally posted by @eiriktsarpalis in dotnet/aspire#6312 (comment)

API Proposal

Exposing the APIs as approved for OrderedDictionary to JsonObject:

namespace System.Text.Json.Nodes;

public partial class JsonObject
{
    public bool TryAdd(string propertyName, JsonNode? value, out int index);
    public bool TryGetPropertyValue(string propertyName, out JsonNode? jsonNode, out int index);
}
@eiriktsarpalis eiriktsarpalis transferred this issue from dotnet/aspire Nov 28, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 28, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Nov 28, 2024
@eiriktsarpalis eiriktsarpalis added area-System.Text.Json and removed untriaged New issue has not been triaged by the area owner needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 28, 2024
@eiriktsarpalis eiriktsarpalis changed the title This workaround requires dual lookup if the key is missing, which suggests to me that a JsonObject.TryAddmethod similar to the one in https://github.com/dotnet/runtime/issues/107947 might be necessary. Add a JsonObject.TryAdd method. Nov 28, 2024
@eiriktsarpalis eiriktsarpalis added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Nov 28, 2024
Copy link
Contributor

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

@eiriktsarpalis eiriktsarpalis self-assigned this Nov 28, 2024
@eiriktsarpalis eiriktsarpalis added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Nov 28, 2024
@terrajobst
Copy link
Member

terrajobst commented Dec 3, 2024

Video

  • Looks good as proposed but we should add an overload of TryAdd without the index for consistency and ergonomics
  • The behavior of what we return for the index should match OrderedDictionary<TKey, TValue>
namespace System.Text.Json.Nodes;

public partial class JsonObject
{
    public bool TryAdd(string propertyName, JsonNode? value);
    public bool TryAdd(string propertyName, JsonNode? value, out int index);
    public bool TryGetPropertyValue(string propertyName, out JsonNode? jsonNode, out int index);
}

@terrajobst terrajobst added the api-approved API was approved in API review, it can be implemented label Dec 3, 2024
@eiriktsarpalis eiriktsarpalis added this to the 10.0.0 milestone Dec 3, 2024
@terrajobst terrajobst removed the api-ready-for-review API is ready for review, it is NOT ready for implementation label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json
Projects
None yet
Development

No branches or pull requests

2 participants