Skip to content
This repository has been archived by the owner. It is now read-only.

Make JsonOutputFormatter use camel case by default #4283

Closed
glennblock opened this issue Mar 14, 2016 · 44 comments

Comments

Projects
None yet
@glennblock
Copy link

commented Mar 14, 2016

It's not the 1990s, everyone expects their JSON to be camel case :-)

Can we get this to be the default so we don't have to do this: https://gist.github.com/shahiddev/e82fea1c85d5e3faa710

I am curious if anyone actually desires pascal case to even be an option.

@davidfowl

This comment has been minimized.

Copy link
Member

commented Mar 14, 2016

/cc @JamesNK

@petarrepac

This comment has been minimized.

Copy link

commented Mar 14, 2016

+1 for camelCase by default

@larsw

This comment has been minimized.

Copy link

commented Mar 14, 2016

Word!

@tmutton

This comment has been minimized.

Copy link

commented Mar 14, 2016

makeJsonCamelCaseAgain

@longchiwen

This comment has been minimized.

Copy link

commented Mar 14, 2016

doit

@khellang

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2016

While we're at it; can we propose to set NullValueHandling to Ignore as well? It's just a waste of bytes, IMO.

I assume that a high percentage of JSON returned from an API is consumed by JavaScript, where both null and undefined are falsy, so if (json.someProperty) { /* do something */ } would have the same results, whether the property is null or just absent.

It should also be no problem if the consumer is using Newtonsoft.Json, where MissingMemberHandling is set to Ignore by default. This would simply set missing properties to default(T).

@petarrepac

This comment has been minimized.

Copy link

commented Mar 14, 2016

@khellang create new issue if you want, let's keep this one focused just on camelCase

@NeelBhatt

This comment has been minimized.

Copy link

commented Mar 14, 2016

Go Go Go just do it 😃

@Gutek

This comment has been minimized.

Copy link

commented Mar 14, 2016

+3.14 :) And more :)

@spboyer

This comment has been minimized.

Copy link

commented Mar 14, 2016

As a compliment to #1765 makes perfect sense. cc/ @danroth27

@shawnwildermuth

This comment has been minimized.

Copy link

commented Mar 14, 2016

+1

1 similar comment
@JalpeshVadgama

This comment has been minimized.

Copy link

commented Mar 14, 2016

+1

@ptrstpp950

This comment has been minimized.

Copy link

commented Mar 14, 2016

Take my vote

@glennblock

This comment has been minimized.

Copy link
Author

commented Mar 14, 2016

Thanks all. Make sure you use the upvote feature if you +1d

@JamesNK

This comment has been minimized.

Copy link
Member

commented Mar 14, 2016

I see +1's are alive and well even after github reactions.

Camel case makes sense when consuming JSON from JavaScript but I think JSON and MVC is more than just JavaScript.

I think the default is better left taking what the user has defined (camel case everywhere would override expicit names specified with JsonPropertyAttribute) and making it clear in the documentation how to config MVC to use camel case everywhere if that is what the user wants. It is a simple change to enable camel case with Json.NET.

@Gutek

This comment has been minimized.

Copy link

commented Mar 14, 2016

I see +1's are alive and well even after github reactions.

Mobile version of page does not allow to add reaction.

@spboyer

This comment has been minimized.

Copy link

commented Mar 14, 2016

@JamesNK is this an 80/20 thing? Like @khellang states above

I assume that a high percentage of JSON returned from an API is consumed by JavaScript...

Or as in the case of #1765

Can this be the default, and have the developer do the 20% exception?

@JamesNK

This comment has been minimized.

Copy link
Member

commented Mar 14, 2016

If you do want to make this change then I think global naming needs a better feature in Json.NET. There are some problems with CamelCasePropertyNamesContractResolver that I haven't yet got around to fixing: it applies to both property names and dictionary names, it doesn't compose well and it always overrides the name from JsonPropertyAttribute.

A better solution would be adding some sort of INamingStrategy exposed on DefaultContractResolver that supports finer grained control.

@bsimser

This comment has been minimized.

Copy link

commented Mar 14, 2016

Where do I submit my PR for this? Make it so!

@Mpdreamz

This comment has been minimized.

Copy link

commented Mar 14, 2016

@JamesNK that would be awesome, we default to camelCase in elasticsearch's .NET client but need some hoops to make sure verbatim properties work e.g dictionaries/json properties/client mappings

https://github.com/elastic/elasticsearch-net/blob/master/src/Nest/CommonAbstractions/SerializationBehavior/GenericJsonConverters/VerbatimDictionaryKeysConverter.cs#L15

And

https://github.com/elastic/elasticsearch-net/blob/master/src/Nest/CommonAbstractions/SerializationBehavior/ElasticContractResolver.cs#L180

A more directly supported approach would be awesome!

@nemi-chand

This comment has been minimized.

Copy link

commented Mar 15, 2016

+1

@Eilon Eilon added this to the 1.0.0 milestone May 26, 2016

@Eilon

This comment has been minimized.

Copy link
Member

commented May 26, 2016

After much deliberation (and discussions with @JamesNK), we have decided by default to support:

camel-case


Source images:

@rynowak

This comment has been minimized.

Copy link
Member

commented May 26, 2016

Yay! we're like everyone else now!

@Eilon

This comment has been minimized.

Copy link
Member

commented May 26, 2016

@rynowak - tell me the truth. Was the ❤️ for the change, or for the amazing MS Paint job?

@JamesNK

This comment has been minimized.

Copy link
Member

commented May 26, 2016

Ah, one issue is INamingStrategy isn't in 8.0.4-beta1. It is sitting in its own branch.

dougbu added a commit to aspnet/Entropy that referenced this issue Jun 9, 2016

React to aspnet/Mvc#4834
- JSON now serialized with camel case
- another part of fix for aspnet/Mvc#4283

dougbu added a commit that referenced this issue Jun 9, 2016

Serialize JSON using camel case by default
- #4283
- update `JsonSerializerSettingsProvider` to choose the `CamelCaseNamingStrategy`

dougbu added a commit that referenced this issue Jun 9, 2016

Serialize JSON using camel case by default
- #4283
- update `JsonSerializerSettingsProvider` to choose the `CamelCaseNamingStrategy`
@dougbu

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

@dougbu dougbu closed this Jun 9, 2016

@dougbu dougbu added 3 - Done and removed 2 - Working labels Jun 9, 2016

@JamesNK

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

I suggest you add a couple of unit tests for behavour of serializing dictionary keys (e.g. IDictionary<string, string>) and properties with JsonProperty("name") to make sure you're happy with how those serialize.

The default constructor for CamelCaseNamingStrategy won't camel case dictionary keys or override explicitly specified names but some tests would verify that MVC expects that camel casing works that way.

https://github.com/JamesNK/Newtonsoft.Json/blob/master/Src/Newtonsoft.Json/Serialization/CamelCaseNamingStrategy.cs

@ScottArbeit

This comment has been minimized.

Copy link

commented Jun 9, 2016

So... if I have existing ASP.NET 4.6 code that reads/writes JSON the default way it's been for many years now, and that JSON is stored as a string, and I want to use .NET Core to read it... will I be able to just read the existing data as-is, or do I have to do a convert-to-camel-case on my data as part of the migration to get it to deserialize into the .NET Core objects? Or is there a PascalCaseNamingStrategy I could use?

@Eilon

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

@SBArbeit I believe you can clear out the naming strategy and that'll work. Or, it might just work anyway, depending on the exact scenario (not everything is always case-sensitive).

@JamesNK

This comment has been minimized.

Copy link
Member

commented Jun 10, 2016

When deserializing Json.NET will first attempt to match a property exactly, e.g. "firstName" in JSON will attempt to be deserialize to a .NET property called firstName, but if it doesn't find that it will do a case insensitive look for properties, and will match a property called FirstName.

You can set NamingStrategy to null.

@ScottArbeit

This comment has been minimized.

Copy link

commented Jun 10, 2016

@Eilon, @JamesNK Thank you for the clarification. That makes it super easy.

@John0King

This comment has been minimized.

Copy link

commented Jun 10, 2016

@JamesNK @rynowak
I really don't like this change, this's will effect thousands of developers who will upgrade to dotnet Core, but seems like many people like came case ,I suggest some feature like this:

[cameCase] //should provide by json.net
public class Persion
{
    public int Id { get; set; }
    public string Name { get; set; }
}

I think this will give more controllable .

Json is not javascript And it not only use by javascript, it's a data formate now use in many languages .
And MVC shouldn't limit the data by default.
Think about this :

// this is a line in project.json 
"Microsoft.AspNetCore.Mvc": "1.0.0-rc2-final",

will be

"microsoft.AspNetCore.Mvc": "1.0.0-rc2-final",

Please rethink about this change.

@JamesNK

This comment has been minimized.

Copy link
Member

commented Jun 10, 2016

[cameCase] //should provide by json.net

There is that feature.

Json is not javascript And it not only use by javascript, it's a data formate now use in many languages
And MVC shouldn't limit the data by default.

All those other languages are either camel casing or snake casing.

It's just a default and it is one line to turn it off.

@jpsingleton

This comment has been minimized.

Copy link

commented Jun 10, 2016

Beyond just casing, I find CamelCasePropertyNamesContractResolver behaves better. I've had issues in the past where the default contract resolver would serialize private properties, might be fixed by now though.

@John0King

This comment has been minimized.

Copy link

commented Jun 10, 2016

@JamesNK Can We use DefaultContractResolver in a few of specific Method when the defalut CamelCaseResolver turn on ?

@Mike-EEE

This comment has been minimized.

Copy link

commented Jun 10, 2016

Have we consulted the Xamarin and enterprise folks on this issue, yet? 😉

So to summarize:

  • C#/Code is PascalCase by default.
  • Serialized version of said code (as data) is camelCase by default.

Please help me see what I am missing here that makes me say that this is inconsistent and fosters (more like festers) context-switching between two the areas of concern.

Maybe I found it here:

It's just a default and it is one line to turn it off.

Thank you for saving my sanity @JamesNK. 😄 That's encouraging. The problem is that C# increasingly appears to be the "odd duck" by holding onto PascalCase as its established default naming convention, and it seems that more and more tech -- MSFT tech, even and this is a good example -- is moving away from it. This is especially so when you view the Azure code samples that are coming out. It seems like everything BUT C# is camelCase.

But alas, we still use C# and hinge upon it for our solutions, generally speaking.

I am all for consistency, but if you are building your application in C#, you simply lose that consistency by deviating from its defaults.

FWIW, I have always felt this way. I was not (and am not) a fan of .NET Configuration's (app/web.config) approach, either.

BTW, this is (yet another reason) why Xaml rocks so hard, as it has 100% parity to what it serializes/deserializes. 😛 The class definition IS the schema.

@glennblock

This comment has been minimized.

Copy link
Author

commented Jun 11, 2016

HTTP is not tied to a particular runtime stack, it is not a serialization
mechanism, it is a message transfer protocol. We are building solutions for
HTTP clients to consume not .NET clients. The majority of the JSON APIs
in existence are camelCase. Why should a client have to know or care that
the backend implementation is .NET. The whole purpose of HTTP is to
abstract away the implementation details of the clients and servers.
On Fri, Jun 10, 2016 at 6:43 AM Mike-EEE notifications@github.com wrote:

Have we consulted the Xamarin and enterprise folks on this issue, yet? 😉

So to summarize:

  • C#/Code is PascalCase by default.
  • Serialized version of said code (as data) is camelCase by default.

Please help me see what I am missing here that makes me say that this is
inconsistent and fosters (more like festers) context-switching between two
the areas of concern.

Maybe I found it here:

It's just a default and it is one line to turn it off.

Thank you for saving my sanity @JamesNK https://github.com/JamesNK. 😄
That's encouraging. The problem is that C# increasingly appears to be the
"odd duck" by holding onto PascalCase as its established default naming
convention, and it seems that more and more tech -- MSFT tech, even and
this is a good example -- is moving away from it. This is especially so
when you view the Azure code samples that are coming out. It seems like
everything BUT C# is camelCase.

But alas, we still use C# and hinge upon it for our solutions, generally
speaking.

I am all for consistency, but if you are building your application in C#,
you simply lose that consistency by deviating from its defaults.

FWIW, I have always felt this way. I was not (and am not) a fan of .NET
Configuration's (app/web.config) approach, either.

BTW, this is (yet another reason) why Xaml rocks so hard, as it has 100%
parity to what it serializes/deserializes. 😛 The class definition IS
the schema.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#4283 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAInROyPkDp6_xQNSCqoFEKji-enJzO2ks5qKWn3gaJpZM4HwDDt
.

@Mike-EEE

This comment has been minimized.

Copy link

commented Jun 11, 2016

Yet we have chosen to tightly-couple our entire design and infrastructure upon HTTP, which seems a little incongruous. Why must we know of HTTP within our code? If there is any abstraction to be done, it would seem that designing a protocol-agnostic system is the place to start. As great as HTTP is with its natural-lending verb and design elements, I still cringe whenever I see anything that requires it (or its concepts) to be in code. I have always found it strange that no one questions why we are coupling design with a protocol.

But I digress. I do appreciate your explanation @glennblock. Thank you for taking the time to do so. I realize I am in the minority here, and I am not looking to reverse community consensus (ok, maybe a little 😄 ). The best I can hope for is to point out the solution inconsistency we have introduced here from a .NET perspective. And although I appreciate the aim of playing along nicely with others going forward, we have done so at a cost towards what has gotten us here to date.

@fatagun

This comment has been minimized.

Copy link

commented May 30, 2018

yeah just do it and let the rest of the world waste their time searching for a solution. Great!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.