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

How useful is C# "dynamic" with System.Text.Json.Nodes.JsonObject? #53195

Closed
steveharter opened this issue May 24, 2021 · 33 comments
Closed

How useful is C# "dynamic" with System.Text.Json.Nodes.JsonObject? #53195

steveharter opened this issue May 24, 2021 · 33 comments
Assignees
Labels
area-System.Text.Json design-discussion Ongoing discussion about design without consensus
Milestone

Comments

@steveharter
Copy link
Member

Currently JsonObject supports C# "dynamic" which primarily means an instance of JsonObject can get\set a property value without having to use a string for the property name.

Although dynamic support has been requested, I believe it was primarily desired as a work-around since System.Text.Json didn't have a writeable DOM feature yet.

Since System.Text.Json now has a writable DOM through JsonNode without dynamic, is the dynamic capability redundant and should it be removed?

In theory, support for dynamic could make code sharing between C# and scripting languaged like JavaScript a bit easier, but what are other benefits are there?

Some negatives of dynamic:

  • Dynamic is slower.
  • There is no IntelliSense or type checking (also the case with non-dynamic cases).
  • The current implementation requires casts to the appropriate type or call to GetValue<T>(), so the copy-paste ability for JavaScript doesn't quite work.
  • Requires System.Text.Json.dll to have a reference to the large System.Linq.Expressions.dll, although the IL Linker removes that if dynamic is not used.
  • Doesn't help with arrays.

Here's a brief sample of dynamic and non-dynamic programming models:

const string Json = "{\"MyNumber\":42, \"MyArray\":[10,11]}";

// dynamic
{
    dynamic obj = JsonNode.Parse(Json);
    int number = (int)obj.MyNumber;
    Debug.Assert(number == 42);
    
    obj.MyString = "Hello";
    Debug.Assert((string)obj.MyString == "Hello");
}

// non-dynamic
{
    JsonObject obj = JsonNode.Parse(Json).AsObject();
    int number = (int)obj["MyNumber"];
    Debug.Assert(number == 42);
    
    obj["MyString"] = "Hello";
    Debug.Assert((string)obj["MyString"] == "Hello");
}

Note that properties on a strongly-typed POCO and collection elements can also be declared as dynamic, and the JsonSerializer can also support dynamic.

@steveharter steveharter added design-discussion Ongoing discussion about design without consensus area-System.Text.Json labels May 24, 2021
@steveharter steveharter added this to the 6.0.0 milestone May 24, 2021
@steveharter steveharter self-assigned this May 24, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label May 24, 2021
@ghost
Copy link

ghost commented May 24, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

Currently JsonObject supports C# "dynamic" which primarily means an instance of JsonObject can get\set a property value without having to use a string for the property name.

Although dynamic support has been requested, I believe it was primarily desired as a work-around since System.Text.Json didn't have a writeable DOM feature yet.

Since System.Text.Json now has a writable DOM through JsonNode without dynamic, is the dynamic capability redundant and should it be removed?

In theory, support for dynamic could make code sharing between C# and scripting languaged like JavaScript a bit easier, but what are other benefits are there?

Some negatives of dynamic:

  • Dynamic is slower.
  • There is no IntelliSense or type checking (also the case with non-dynamic cases).
  • The current implementation requires casts to the appropriate type or call to GetValue<T>(), so the copy-paste ability for JavaScript doesn't quite work.
  • Requires System.Text.Json.dll to have a reference to the large System.Linq.Expressions.dll, although the IL Linker removes that if dynamic is not used.
  • Doesn't help with arrays.

Here's a brief sample of dynamic and non-dynamic programming models:

const string Json = "{\"MyNumber\":42, \"MyArray\":[10,11]}";

// dynamic
{
    dynamic obj = JsonNode.Parse(Json);
    int number = (int)obj.MyNumber;
    Debug.Assert(number == 42);
    
    obj.MyString = "Hello";
    Debug.Assert((string)obj.MyString == "Hello");
}

// non-dynamic
{
    JsonObject obj = JsonNode.Parse(Json).AsObject();
    int number = (int)obj["MyNumber"];
    Debug.Assert(number == 42);
    
    obj["MyString"] = "Hello";
    Debug.Assert((string)obj["MyString"] == "Hello");
}

Note that properties on a strongly-typed POCO and collection elements can also be declared as dynamic, and the JsonSerializer can also support dynamic.

Author: steveharter
Assignees: steveharter
Labels:

Design Discussion, area-System.Text.Json

Milestone: 6.0.0

@FilipToth
Copy link
Contributor

Well, in my opinion, dynamic makes it a lot cleaner to read/write DOM in applications where speed isn’t a major issue.

@davidfowl
Copy link
Member

cc @JamesNK

@jkotas
Copy link
Member

jkotas commented May 25, 2021

Requires System.Text.Json.dll to have a reference to the large System.Linq.Expressions.dll, although the IL Linker removes that if dynamic is not used.

System.Linq.Expression is "archived component". dynamic as it exist today is effectively deprecated tech. It is not good for new tech to take dependency on effectively deprecated tech.

@FilipToth
Copy link
Contributor

FilipToth commented May 25, 2021

@jkotas are there any plans to update it? Aren’t dynamics a good way to interop with non-statically typed languages?

@jkotas
Copy link
Member

jkotas commented May 25, 2021

I am not aware of any plans around better dynamic. I agree that it is attractive for certain scenarios and there may be a way to reinvent it to be simple, lean, fast, and without the problematic heavy System.Linq.Expression dependency, but that is completely speculative at this point.

More discussion on this is at dotnet/designs#163 (comment)

@FilipToth
Copy link
Contributor

FilipToth commented May 25, 2021

Maybe in later dotnet versions then. Thanks.

@steveharter
Copy link
Member Author

@KrzysztofCwalina comments on supporting a better version of dynamic (with IntelliSense perhaps)?

@davidfowl
Copy link
Member

@CyrusNajmabadi how crazy would it be to do something with embedded languages and JSON? Using the JSON to drive IntelliSense of the dynamic object being returned from these APIs? Of course it's easy when you have a JSON literal but I wonder if the schema for IntelliSense could come from outside of the C# type system and feed into it (I guess it's effectively F# type providers).

@JamesNK
Copy link
Member

JamesNK commented May 25, 2021

My feedback is the same:

  • I don't see many people using dynamic with Json.NET (however I don't go looking for this kind of information).
  • I don't think it is a hugely valuable feature.
  • I wouldn't recommend making dynamic a focus of how a JSON DOM is used and designing around it, but if you want to make it an option, and you're ok with the complexity that adds, then go ahead.

@stonstad
Copy link

stonstad commented May 27, 2021

I prefer the dynamic approach because it is implementation agnostic after deserialization occurs.

As a secondary reason, there are common workflows using RESTful WebAPI to send and receive JSON via dynamic. I think this speaks to the importance of supporting dynamic deserialization up and down the stack.

@stonstad
Copy link

stonstad commented May 27, 2021

Maybe in later dotnet versions then. Thanks.

My understanding of this feature (deserialization to dynamic) is that it was supposed to ship in .NET 6.0, having missed the cut-off for 5.0.

@imba-tjd
Copy link
Contributor

System.Linq.Expression is "archived component".

Unrelated to this issue, is there a list of "archived component" to let us know what are not in actively developed?

@KalleOlaviNiemitalo
Copy link

There was apparently a list of archived components in #27790, but I don't know if more components have been archived since then.

@jkotas
Copy link
Member

jkotas commented May 27, 2021

Archived components are tagged in https://github.com/dotnet/runtime/blob/main/docs/area-owners.md

@gregsdennis
Copy link
Contributor

I prefer a strongly-typed approach. In Manatee.Json (now deprecated), I set up implicit casts from compatible types into my JsonValue type, which was a holder for any JSON value. It supported

  • numeric types
  • booleans
  • strings
  • JsonObject
  • JsonArray

Limiting it to these types keeps a nice strong type model but also allows for fairly dynamic building.

var obj = new JsonObject
    {
        ["key"] = "value",
        ["otherKey"] = 9
    }
var array = new JsonArray { "value", 9, obj };

Using implicit casts this way may give some people (cough E. Lippert cough) a bit of heartburn, but it makes for some very readable code. It's not 100% copy-paste-able from JS, but it's pretty close.

Secondarily, I tend to stay as far away from dynamic as possible. It has its uses, but it's not ideal for everyday use. Generally, there's a better solution; you just have to find it.

@lukasf
Copy link

lukasf commented Jun 11, 2021

I really like the approach from @gregsdennis! But I assume not everyone likes implicit casts.

It would be really cool if we could define implicit casts in extension methods. Then we could have an extension class in a namespace System.Text.Json.Nodes.Implicit. If we'd include that, we'd get implicit casting, allowing fluid JS-like syntax without all the downsides of dynamic. Users who don't include it won't have to worry about implicit casts happening without notice.

@KrzysztofCwalina
Copy link
Member

I think using dynamic for writing/mutating the DOM is clunky, but I think it's very useful for reading. We have many Azure SDK libraries that return raw JSON payloads (as opposed to deserialized "model" types) and we are hoping to use JsonNode with dynamic to access the returned data, e.g.

dynamic json = client.GetSomething(...);
int x = json.Foo.Bar.Baz;

It does worry me that 'dynamic' is considered to be archived.

@stephentoub
Copy link
Member

It does worry me that 'dynamic' is considered to be archived.

Just to clarify, Krzysztof, it worries you in general about dynamic (e.g. that it's not being invested in), or it worries you about this new JSON support taking a dependency on such a thing?

@gregsdennis
Copy link
Contributor

I know the question wasn't directed at me, but I'm definitely worried that the JSON API could be dependent on dynamic. It's widely regarded as a code smell and should be considered only as a last option.

@KrzysztofCwalina
Copy link
Member

KrzysztofCwalina commented Jun 14, 2021

Just to clarify, Krzysztof, it worries you in general about dynamic (e.g. that it's not being invested in), or it worries you about this new JSON support taking a dependency on such a thing?

Both. New JSON APIs should not depend on "archived" technologies. But on the other hand, the web is full of untyped data and it would be great if BCL had a modern tech to access such data.

@steveharter
Copy link
Member Author

To recap feedback:

  • "dynamic" is somewhat "archived" technology and there may be a newer flavor of it coming up.
    • This potential newer flavor may have improved performance as well as new features.
    • It is not 100% certain to me if code that uses "dynamic" today will be completely forward-compatible with any newer "dynamic" flavor, thus introducing some risk in adopting it at this time.
  • Currently "dynamic" is useful sugar in certain scenarios, but otherwise not a high priority or high consumption feature.
  • The new JsonNode feature avoids most of the scenarios that made "dynamic" useful previously (that is, a writable DOM).

Based on this, I'm inclined to suggest removing the "dynamic" support for V6 unless I hear feedback otherwise from @KrzysztofCwalina or others.

@stephentoub
Copy link
Member

That sounds like the right decision to me, @steveharter.

@KrzysztofCwalina
Copy link
Member

KrzysztofCwalina commented Jun 16, 2021

Who's working on the "This potential newer flavor" of dynamic? I do think we need it at some point. It might be a syntactic sugar, but the current syntax obscures the user intent enough that our current plan for untyped APIs in the Azure SDK might not be viable without it. After all C# is just syntactic sugar for IL, and you would not enjoy coding in IL, would you? :-)

cc: @MadsTorgersen

@KrzysztofCwalina
Copy link
Member

Having said that, I would totally prefer a new dynamic flavor. There is no reason to involve the DLR into what amounts to syntactic sugar over dictionary access. I would prefer if dynamic was implemented as a set of attributes applied to simple lookup and cast APIs.

@John0King
Copy link

@steveharter

what are being "archived" ? dynamic or System.Linq.Expression ?

@jkotas
Copy link
Member

jkotas commented Aug 12, 2021

@John0King Both dynamic (as it exists today) and System.Linq.Expression are archived components. See #27790 for the discussion.

@John0King
Copy link

John0King commented Aug 30, 2021

@jkotas Linq and Expression are largely used in many many place, such as EF Core, And I believe many people do not know about the talking even start, your team should have a announcement to the community that you are discussing about those , I find that any news in https://github.com/dotnet/announcements/issues be noticed by me is also too late, you already done the change and is waiting for feedback about the new change.

In fact, today's .Join method in Linq is not easy to use compare a .LeftJoin() or .RightJoin expression, and With all async mode , there no async version of ToListAsync() ToArrayAsync method.
and make those things archived, seems declare their death, but where's the replacement ?

IMO, you can declare something should not be used , but there must be a replacement, otherwise, how can anyone who used those code and move forward together with the .Net version ? Please don't drop developers ! there no more developers of .net to drop anymore!

@jkotas
Copy link
Member

jkotas commented Aug 30, 2021

how can anyone who used those code and move forward together with the .Net version ?

We are not deleting any of these APIs. The existing code that uses these APIs can be moved to new .Net versions just fine.

@John0King
Copy link

@jkotas I think "archived" means: ready for obsolete and deprecated

@stephentoub
Copy link
Member

I think "archived" means: ready for obsolete and deprecated

It does not. The meaning as we use it is spelled out in the README for the relevant library:
https://github.com/dotnet/runtime/blob/main/src/libraries/System.Linq.Expressions/README.md
"The library and supporting language features are mature and no longer evolving, and the risk of code change likely exceeds the benefit. We will consider changes that address significant bugs or regressions, or changes that are necessary to continue shipping the binaries. Other changes will be rejected."

@davidikin45
Copy link

davidikin45 commented Sep 9, 2021

Wasn't the whole point of #29690 to allow WebAPI to accept untyped json payloads and access them easily. e.g model.Property1? Seems strange you would regress this functionality previously provided by Newtonsoft.Json

[HttpPost] public async Task<IActionResult> SubmitAsync(dynamic model)

@gregsdennis
Copy link
Contributor

Yes, but it was concluded that dynamic is the wrong way to support this as it subverts the type system.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json design-discussion Ongoing discussion about design without consensus
Projects
None yet
Development

No branches or pull requests