Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

[Discussion] MVC now serializes JSON with camel case names by default #4842

Closed
dougbu opened this issue Jun 9, 2016 · 125 comments
Closed

[Discussion] MVC now serializes JSON with camel case names by default #4842

dougbu opened this issue Jun 9, 2016 · 125 comments

Comments

@dougbu
Copy link
Member

dougbu commented Jun 9, 2016

In previous milestones, MVC's JSON serialization used Json.NET's default naming convention. This maintained C# property names in the JSON.

In 1.0.0, MVC uses camel case names by default. This matches most JSON naming conventions.

Potential compatibility breaks

Applications which depend on the exact bytes sent over the wire or that include code such as

dynamic d = JObject.Parse(body);

may need to be adjusted.

To restore previous naming strategy

If you have case-sensitive clients that cannot be easily updated, change your Startup from

    services.AddMvc();

to

    services
        .AddMvc()
        .AddJsonOptions(options => options.SerializerSettings.ContractResolver = new DefaultContractResolver());
Example
Before
public class Person
{
    public int Id { get; set; }

    public string FullName { get; set; }
}

Would serialize to

{"Id":9000,"FullName":"John Smith"}
After

The same model will serialize to

{"id":9000,"fullName":"John Smith"}

Note the initial lowercase letters.

@dougbu
Copy link
Member Author

dougbu commented Jun 9, 2016

This is the discussion issue for aspnet/Announcements#194.

@dougbu
Copy link
Member Author

dougbu commented Jun 9, 2016

See also the original issue we fixed in 1.0.0: #4283

@clarkis117
Copy link

What would this mean if we use a JavaScript framework like Knockout.js to map JSON inputs to view models in our client side code? And is there a way to change this back to Json.NET's default?

@sandorfr
Copy link

sandorfr commented Jun 9, 2016

@clarkis117 , yes there is :

services.AddMvc().AddJsonOptions(options =>
            {
                options.SerializerSettings.ContractResolver = new DefaultContractResolver();
            });

This is already available so you can define your settings right now.

@brent-russell
Copy link

I like this change, but -

I think it needs to be configurable to allow developers to choose the serialization naming convention to be used.

  1. It's nice to let people choose
  2. This change adds A LOT of work for me and others who will upgrade from RC1
  3. Better backwards compatibility and upgrade path with existing custom libraries that utilized Json.NET

@sandorfr
Copy link

sandorfr commented Jun 9, 2016

@brent-russell this is configurable ;)

@dougbu
Copy link
Member Author

dougbu commented Jun 9, 2016

@brent-russel, as @sandorfr showed, this is quite configurable. What exactly is missing from your perspective?

@brent-russell
Copy link

@sandorft @dougbu Oh, that's great!

Just communication and documentation then I guess. The announcement didn't hint at configuration at all. Most other announcements that introduce breaking changes provide instructions on how to implement or obtain pre-existing behavior.

Thanks!

@Eilon Eilon modified the milestones: Discussions, 1.0.0 Jun 10, 2016
@lucamorelli
Copy link

hmmm ... I use typescript and tools like create intellisense file from web essentials to map automatically a csharp in the backend to a interface definition in the client.
If you use whatever tool to make automatically server-client entities in typescript, this way by default are not going to work

@clarkis117
Copy link

@lucamorelli Yes, this is one of the problems that I foresee running into with using Typescript tools like Typewriter. Potentially I'd have to change to change parts of my Typewriter model generation code to camel case, which would break significant portions of the manually written client side code.

@John0King
Copy link

I really don't like this change, it will effect thousand of developers. I suggestion some feature like this:

[camelCase] // provide by json.net
public class Person
{
    public int Id { get; set; }

    public string FullName { get; set; }
}

this will give us more controllable of our code.

Json is not javascript, and it not only use by javascipt. It's a data formate now use in many languages,
Please rethink about it.

@poke
Copy link
Contributor

poke commented Jun 10, 2016

@John0King As shown above, this is already controllable. You can always revert to the old behavior if you prefer that, or go the explicit route and annotate the class with serialization attributes.

This change of the default behavior does make a lot of sense. Most other languages nowadays consistenly prefer camel casing for all members; it’s just C# that’s odd here with its pascal case properties. So languages consuming JSON are more likely to feel familar with camel cased JSON properties.

I welcome this change, thanks!

@sandorfr
Copy link

@John0King from my experience the Attribute thing is to be kept minimal, it's easily forgotten. The way to go is AddJsonOptions.

For the "I like this better", I personally don't care, but I think using AddJsonOptions should be a recommend practice not matter you want the default or another behavior.

@jpsingleton
Copy link

Looks good to me. Saves having to set CamelCasePropertyNamesContractResolver in Web API.

Here is an example of doing this from the old Web API on the full .NET Framework, but I'm planning on porting this to Core after it RTMs.

@Mike-E-angelo
Copy link

Buggah... I suck at life, and replied to the wrong thread on #4283. Boh.

Anyways, I again find myself in the minority here. I also believe I speak for a lot of the native/Xamarin/enterprise developers that wish to keep consistency between code (C#) and the data that represents that code.

I would feel better if this change default was hinged to a particular "style" setting/attribute somehow, meaning that in native/enterprise applications, PascalCase is the default, and in web applications camelCase is the default.

@ciel
Copy link

ciel commented Jun 10, 2016

It basically is, though. It's literally one line of code to switch it
around.
On Jun 10, 2016 8:49 AM, "Mike-EEE" notifications@github.com wrote:

Buggah... I suck at life, and replied to the wrong thread on #4283
#4283. Boh.

Anyways, I again find myself in the minority here. I also believe I speak
for a lot of the native/Xamarin/enterprise developers that wish to keep
consistency between code (C#) and the data that represents that code.

I would feel better if this change default was hinged to a particular
"style" setting/attribute somehow, meaning that in native/enterprise
applications, PascalCase is the default, and in web applications camelCase
is the default.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#4842 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAavtJZY3M0r8OFfYHyUPPNBLU0OeC8Sks5qKWtVgaJpZM4IyfXK
.

@Mike-E-angelo
Copy link

Yes indeed @ciel I understand that, but what this decision says is that by default the camelCase is the better/preferred format, whereas PascalCase is not. So what does this say about C#, upon which (ironically) the serializer is written in?

This introduces confusion and context-switching into the process. I hope that you can understand my viewpoint and concern here.

@ciel
Copy link

ciel commented Jun 10, 2016

I understand your standpoint and concern, and to an extent I share it. But
I also see the ease of switching makes it a non-issue overall. It could be
even further remedied by just including the code to control the format as
part of the default templates.
On Jun 10, 2016 8:54 AM, "Mike-EEE" notifications@github.com wrote:

Yes indeed @ciel https://github.com/ciel I understand that, but what
this decision says is that by default the camelCase is the better/preferred
format, whereas PascalCase is not. So what does this say about C#, upon
which (ironically) the serializer is written in?

This introduces confusion and context-switching into the process. I hope
that you can understand my viewpoint and concern here.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4842 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAavtK-sPphSFo0gUngfnPV2Foy7oE8fks5qKWyegaJpZM4IyfXK
.

@ciel
Copy link

ciel commented Jun 10, 2016

I don't think it is a good or wise decision. But, I also don't have my
fingers in dozens of projects. I have faith that the people who do made
this change for a good reason. That's all. I'm not discounting what you're
saying.
On Jun 10, 2016 8:56 AM, "Stacey Lynn" stacey.cielia.lynn@gmail.com wrote:

I understand your standpoint and concern, and to an extent I share it. But
I also see the ease of switching makes it a non-issue overall. It could be
even further remedied by just including the code to control the format as
part of the default templates.
On Jun 10, 2016 8:54 AM, "Mike-EEE" notifications@github.com wrote:

Yes indeed @ciel https://github.com/ciel I understand that, but what
this decision says is that by default the camelCase is the better/preferred
format, whereas PascalCase is not. So what does this say about C#, upon
which (ironically) the serializer is written in?

This introduces confusion and context-switching into the process. I hope
that you can understand my viewpoint and concern here.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#4842 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAavtK-sPphSFo0gUngfnPV2Foy7oE8fks5qKWyegaJpZM4IyfXK
.

@0xdeafcafe
Copy link

Also, the general naming convention for data names in json on the web now is snake_case, and not camelCase. Seems a little weird to set the default to be something that is in the middle of the c#'s naming convention, and the most common json naming convention.

@Mike-E-angelo
Copy link

That's all. I'm not discounting what you're saying.

Cool thank you for that @ciel. :)

And yes, I can see why it's a non-issue. I mean, people say the same thing about xml vs. json. 😉 To me it all comes down to consistency. That is why I wonder if Xamarin/enterprise side of the aisle was consulted with this as this makes their (or our 😄 ) side inconsistent by default now.

@Mike-E-angelo
Copy link

Mike-E-angelo commented Jun 10, 2016

Seems a little weird to set the default to be something that is in the middle of the c#'s naming convention, and the most common json naming convention

I hear you @0xdeafcafe (awesome name btw LOL!), but what we seems to get lost here is that there is a history and legacy -- not to mention an entire enterprise -- that you also have to consider here with MSFT. This should be the learning lesson of project.json (as I tried to point this out back in the day, as well).

By itself, in the ".NET Core" or "web" way of things, this makes sense. But when you shine this through the prism of MSFT's history and other commitments, you run into problems.

I think this just goes to further the need to recognize the schism/chasm that is within MSFT at present: enterprise (legacy/native) and web (new and shiny). I don't want to infringe upon your standards (or creativity/innovation as what is going on here is great!), but let's not infringe upon others while we're at it.

@0xdeafcafe
Copy link

0xdeafcafe commented Jun 10, 2016

@Mike-EEE I guess, I just don't get why they didn't leave it as is, and not annoy anyone (as we're all used to writing custom converts/setting attributes/custom serialization settings). Instead of changing it to something in the middle that is just going to irritate both sides, and only really please those few in the middle. On top of that, most "enterprise" companies either don't use JSON, or use PascalCase if they do.

@Mike-E-angelo
Copy link

You know @0xdeafcafe ... I actually just re-read what you wrote and have to say that I totally misunderstood you! Yes, you are correct. This does seem to be a "middle" ground play, for lack of a better word.

And the web evolves so fast, who knows what format will be the flavor of the month, next. 😉

Most "enterprise" companies either don't use JSON, or use PascalCase if they do.

Very true! We should say they don't use JSON... YET. As .NET Core moves towards making this a requirement. 😛 But I am very much a proponent of enabling developers and organizations to "bring your own serializer" and allow them to use the data formats (and corresponding tools) that make them the most productive. A notion that hasn't quite caught on here yet.

My position is that all decisions should "protect the investment" and brand of MSFT. That means innovate and appeal to other methods/technologies (as VS Code is doing a great job of doing), but do so in light of the history and tech that got us here.

@dotnetchris
Copy link

This change is probably correct. However the execution of this is absolutely insane. Right in the middle of RC2 and RTM is when you decide you're going to break every single application that uses C# Pascal cased property names because that's what the data serialized as.

Is this the decision making process going there?

hydra and hercules

@MaximRouiller
Copy link

@bwatts did you open an issue on https://github.com/JamesNK/Newtonsoft.Json?

If it's a bug, @JamesNK should be informed. I've looked quickly at the repository and it doesn't look like this issue has been logged before.

@bwatts
Copy link

bwatts commented Sep 15, 2016

@MaximRouiller I wasn't sure if it is a bug or just an unfortunate feature collision. I'll open an issue over there anyhow to be sure.

@khellang
Copy link
Contributor

khellang commented Sep 16, 2016

@bwatts @MaximRouiller Camel-case dictionary keys is something that needs to be opted into. If you want to tweak the settings, you can set the JsonOptions:

services.AddMvc().AddJsonOptions(x =>
{
    x.SerializerSettings.ContractResolver = new DefaultContractResolver
    {
        NamingStrategy = new CamelCaseNamingStrategy
        {
            ProcessDictionaryKeys = false // this is the default.
        }
    };
});

As you can see, the MVC defaults are the same as Newtonsoft.Json defaults, which should keep dictionary keys the same.

@MaximRouiller
Copy link

Thanks @khellang for the clarification! Didn't know about that specific feature.

@bwatts
Copy link

bwatts commented Sep 16, 2016

@khellang I found NamingStrategy and ProcessDictionaryKeys while investigating, but this fiddle seems to contradict that default. Thoughts?

@khellang
Copy link
Contributor

@bwatts Uuh. CamelCasePropertyNamesContractResolver isn't the same as DefaultContractResolver 😉

This fiddle shows that it works 😄

@kijanawoodard
Copy link

kijanawoodard commented Sep 16, 2016

I'm using this: http://stackoverflow.com/a/24226442/214073.
Not sure if it's still required, but works.

@bwatts
Copy link

bwatts commented Sep 16, 2016

@khellang isn't the point of the MVC default-to-camel-casing to make the CamelCasePropertyNamesContractResolver the default?

@khellang
Copy link
Contributor

khellang commented Sep 16, 2016

@bwatts No, because as you've discovered, CamelCasePropertyNamesContractResolver also makes dictionary keys camel-cased. That's whay @JamesNK introduced the NamingStrategy class.

MVC uses the DefaultContractResolver with NamingStrategy set to CamelCaseNamingStrategy, which has ProcessDictionaryKeys set to false by default.

@bwatts
Copy link

bwatts commented Sep 16, 2016

@khellang got it - so it seems I shouldn't be using CamelCasePropertyNamesContractResolver at all then. I didn't realize.

@gr3ysky
Copy link

gr3ysky commented Oct 25, 2016

This change is really ridicilious. You need to tell mvc to use default serializer to get the default behavior. This is why microsoft is not liked by many.

@kijanawoodard
Copy link

kijanawoodard commented Oct 25, 2016

@gr3ysky The problem is "the default for what scenario"?

It's all a matter of choices.
At least now, we know what the choices are more or less when they happen and we have a way to tailor things to our liking.

@gr3ysky
Copy link

gr3ysky commented Oct 25, 2016

The default is the f.cking default. No need to argue on what is default. It shouldnt change the property names. It is just like start menu issue. I have been developing mvc over six years. It was pascal case till now. And just check the code to get old behaviour. And thats silly to say use the default behavior. Right?

@fhelwanger
Copy link

@gr3ysky Did you read #4842 (comment)?

@kijanawoodard
Copy link

Further, you're arguing defaults should never change? This is a major version.
First thing I do on every project is change the "default" so I get "proper serialization" for javascript.

What's "right" is a judgement call, not an objective fact.

@MaximRouiller
Copy link

The new "Default" for serialization in JSON is now lower case.

The model binders for ASP.NET Core will interpret lower-case/upper-case without any issues so this is a non-issue for "current behavior" of the API.

So let's take a look at what scenarios you would need to change it.

  1. New Project
  2. Porting to .NET Core

Here are the solutions:

  1. Either the invoking client adapts to the API it invokes and parse the data properly or you adapt to the client and do a one line change of code to suit the invoker. Either way, minor change.
  2. You are porting an API from .NET Framework to .NET Core. So between the whole HttpApplication going away, the application lifecycle being modular, project.json, WebAPI/MVC being merged, Kestrel as a running host, EF Core != EF6, lack of Azure integrations, dependency injections as a first class citizen, no oData support yet, missing libraries that haven't converted to Core... and what is a deal breaker is ... default serialization for JSON.

It's a major rewrite of a framework. You can't expect to copy/paste your code and expect everything to be feature equal. I have more complaints about EF Core right now than the default serialization for JSON which can be reverted in one line of code. As mentioned before, there's only Microsoft that defaults the JSON serialization to Pascal Case. Now we are "back to normal" in the web sphere.

If you want to go back to the good old days when you are porting an application, here's how:

services.AddMvc().AddJsonOptions(options =>
            {
                options.SerializerSettings.ContractResolver = new DefaultContractResolver();
            });

@Eilon
Copy link
Member

Eilon commented Jun 9, 2017

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.

@Eilon Eilon closed this as completed Jun 9, 2017
@Mike-E-angelo
Copy link

If I could, I wanted to thank everyone for the respectful and thoughtful discussion on this memorable thread, especially when it turns out I had no idea what I was talking about. 😛 (or to be more accurate, did not completely understand the issue at hand and misunderstood its actual implications).

I'd also like to send a special shout out to @bwatts for his "no good deed" comment above as well. I didn't really understand it at the time, but there have been been numerous instances since then in other discussions throughout GitHub where I have been brought to bear the full appreciation of that statement, LOL. If it helps at all, rest assured that I have gotten my due on the flip side. 😆

Anyways, thought I would share. And no, I can't help it. 👍

@maxisam
Copy link

maxisam commented May 23, 2018

It is interesting that if an object is not strong type like object or dynamic, it will not be converted to camelcase. But it can be fixed by

     .AddJsonOptions(opts =>
      {
            opts.SerializerSettings.ContractResolver = new CamelCasePropertyNamesContractResolver();
      });

@khellang
Copy link
Contributor

khellang commented May 23, 2018

Or by using the DefaultContractResolver and setting ProcessDictionaryKeys = true (which I think is recommended):

.AddJsonFormatters(json =>
{
    json.ContractResolver = new DefaultContractResolver
    {
        NamingStrategy = new CamelCaseNamingStrategy
        {
            ProcessDictionaryKeys = true
        }
    };
});

@QuAzI
Copy link

QuAzI commented Oct 25, 2018

Wow! I found this ugly bug! And it is a really bug!
This behavior broke the data exchange between C# and JS!

  1. All properties in JS must be fixed after migration from ASP.NET
  2. Doesn't have backward compatibility (to restore broken case) and can't be returned AS IS to the ASP.NET Controller
public class TestModel
{
  public int Id { get; set;}
  public Dictionary<string, string> Settings { get; set; }
}

Simple method to use model

<script>
    var model = @Html.Raw(Json.Serialize(Model)); // case is broken there when ASP.NET Core used

    // some methods to bind inputs with model

    $('form').submit(function (e) {
        e.preventDefault();
        $.ajax({
            url: this.action,
            data: JSON.stringify(model),
            type: "POST",
            beforeSend: function (xhr) {
                xhr.setRequestHeader("Content-type", "application/json");
            },
            async: false
        });
    });
</script>

Simple method which must save returned model

[HttpPost]
public ActionResult YmlConfiguration([FromBody]TestModel config)
{
    // config = null and NullReferenceException fired there because case broken!
}

It is improbable to change names at serialization! Why?

@MaximRouiller
Copy link

@QuAzI If you want to restore the older behavior of keeping the casing, see the comments above. They show you many ways to resolve that issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests