aspnet / Mvc

[Archived] ASP.NET Core MVC is a model view controller framework for building dynamic web sites with clean separation of concerns, including the merged MVC, Web API, and Web Pages w/ Razor. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
5.62k stars 2.14k forks source link

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

Closed dougbu closed 7 years ago

dougbu commented 8 years ago

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.

bwatts commented 8 years ago

Is it possible, just possible, that the .NET renaissance could lead to an increased awareness of other communities? Perhaps we could admit that perpetuating cultural ignorance (in this case, the culture around JSON) might be counter to the strategy of an inclusive .NET?

ghost commented 8 years ago

@khellang, I agree that the reason for the change (it's what you want 99% of the time), is a damn strong argument.

I guess some of us have PascalCase ingrained in our brain when working with .NET and the new default will feel wrong, but the final decision should be what is best for the ASP.NET Core MVC ecosystem.

Just wanted to put my 2¢ in and voice my concerns regarding this decision.

Have a nice day guys!

kwaclaw commented 8 years ago

I am reading this thread and I am thinking: Whats the big deal? All options are still available.

If I had to issue an opinion: JSON is based on Javascript and it makes sense to follow the dominant conventions in the Javascript world. It is irrelevant between which types of systems JSON is used for data serialization.

khellang commented 8 years ago

@Mike-EEE

You mean with its existing inconsistent, conflicting, and contradictory defaults.

Why should there be one default for all of .NET? The way it is today, on the web, .NET is the odd one out.

Does that same style guide suggest comments in its JSON? :angel:

Why would it? There's no comments in the JSON spec.

It's about time MVC ditched the JavaScript. :stuck_out_tongue:

I really don't know what to say here... It seems you've been living under a rock (or at least far away from the web) for a while :stuck_out_tongue_closed_eyes: I see your argument though; why can't we just close out the rest of the world and live in our own warm and cozy little .NET bubble? Let's see how long that lasts. People are already fleeing :smile:

The same case can be made for the reverse. :)

But in your quote, you left out an imortant part:

Who do you want to optimize for? The 20% or the 80%?

Do you want to stick with the existing defaults and force 80% of people, maybe also coming from other languages/stacks, to change them? Or do you change the defaults and make the remaining 20% change them?

Again, this is a MSFT product based in MSFT history and this must be accounted for when considering these sorts of changes.

If we'd stuck with MSFT defaults and MSFT history, we'd still be sabotaging the web with IE6. Let's not forget and instead learn from history.

Has project.json taught us nothing?

I don't think (reverting) project.json ever was about angle brackets vs curly braces, so I'm not sure what it has to do with this... All its keys were camelCased. Are you saying that's why we're going back? Cause that would be pretty naïve.

Mike-E-angelo commented 8 years ago

Whats the big deal?

It's the principle of the matter. :)

JSON is based on Javascript and it makes sense to follow the dominant conventions in the Javascript world

Again this makes sense if you are building a non-MSFT and non-.NET product that has nothing to do with C# or anything .NET-related. However (and unfortunately) this is not the case as .NET Core is indeed attached to all of the history of .NET and MSFT.

project.json was a good example of this.

Mike-E-angelo commented 8 years ago

Why should there be one default for all of .NET? The way it is today, on the web, .NET is the odd one out.

100% correct if you look at it from a web-perspective, 100%-wrong if you look at it from a .NET (native/enterprise) perspective. :)

Why would it? There's no comments in the JSON spec.

100% correct, yet we have modified/adjusted JSON.NET and project.json to allow them (this was a prod towards deviating from standards, that obviously didn't take!).

I really don't know what to say here... It seems you've been living under a rock (or at least far away from the web) for a while :stuck_out_tongue_closed_eyes:

Hahaha... no I have been having to endure putting up w/ HTML5/JavaScript polluting our ecosystem since Silverlight bit the dust. :smile: ... or i should say: :frowning: :frowning: :frowning:

why can't we just close out the rest of the world and live in our own warm and cozy little .NET bubble?

While simultaneously reaching out to and embracing external developers, technologies, and communities. I agree! It's much easier said than done.

Do you want to stick with the existing defaults and force 80% of people, maybe also coming from other languages/stacks, to change them? Or do you change the defaults and make the remaining 20% change them?

Well who is the 80%? From the web/adoption perspective the 80% is the webdev/JavaScript/HTML5 "web designer" crowd (that Visual Studio Code targets). From the historical MSFT perspective, 80% would be the native/enterprise crowd. Guess which one of these has put hundreds of millions (if not billions) of dollars into MSFT's coffers? :)

If we'd stuck with MSFT defaults and MSFT history, we'd still be sabotaging the web with IE6. Let's not forget and learn from history.

Fair enough (dang you bring up great points! I haven't quoted so much on GitHub as this post). I suppose I should have said MSFT .NET/Developer history. This has been very successful and has done much better than IE6 and a lot of other products.

Are you saying that's why we're going back?

project.json overlapped with MSBuild and Visual Studio tooling in so many ways. This has been and was brought up for more than a year as votes and with issues in associated repos (example), but were essentially disregards and ignored. It wasn't until the Xamarin acquisition and the enterprise groups starting to catch wind of project.json did everyone suddenly realize that there were a lot of scenarios (read: history and legacy) to account for that project.json simply did not fulfill, hence the reversal.

I am using that as a premier example of saying that the efforts done here impact more than just this project, and we should be mindful of the entire .NET developer ecosystem and be considerate, accordingly.

khellang commented 8 years ago

100% correct if you look at it from a web-perspective, 100%-wrong if you look at it from a .NET (native/enterprise) perspective. :)

Last time I checked, MVC was a web framework.

Bike Shed

I'm out :wave: This thread makes me want to leave .NET :sob:

Mike-E-angelo commented 8 years ago

Last time I checked, MVC was a web framework.

A .NET web framework, written in .NET languages (which follow .NET guidelines). :)

This thread makes me want to leave .NET :sob:

:frowning: Really appreciate the dialogue and thoughts, though. :+1:

bwatts commented 8 years ago

I was thinking maybe this should be red:

Bikeshedding

Eilon commented 8 years ago

Folks, at this time we are not planning to change this decision. We feel we made the decision that overall makes things better for ASP.NET, MVC, and .NET. For those who are negatively affected by this decision, it's a simple switch to change the behavior back:

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

Given that we are not changing the behavior, we would like to keep this thread focused around the change, as opposed to whether it's a good change or not.

John0King commented 8 years ago

CamelCase PascalCase I really don't care, I think the goal is to get the data I want, if the client need camelCase ,then give it with camelCase, if client need PascalCase then give it with PascalCase , if client need both , give it with the the client need, someone like( or need) camelCase you can write camelCase Object or Use [JsonProperty("myName")] on MyName property . Now default behavior is to convert the code you write and into something else, It's like you write game, when press up key, player go up,press down key,player go down, but when you run this game, you press up key ,player go down, is this really you want to be with you in next 10 years ? someone also say other language all use camelCase json , I'm not a expert but is there any language serialize json won't use the same name as you write? And i'm sure it will generate PascalCase json when you write object in PascalCase. Other side: if you write a C# object

public class MyData
{
    public string WWTUrl { get; set; }
    public string W3CUrl{ get; set;}
}

serialize to

{
    "wWTUrl":"http://www.worldwidetelescope.org/",
    "w3CUrl":"http://www.w3.org/"
}

Is that will feel good ? sorry for my bad english

Mike-E-angelo commented 8 years ago

For those who are negatively affected by this decision, it's a simple switch to change the behavior back:

Totally missing the point here. :) But that was the case for project.json as well. Sooooo... I will just let it ride until someone with a little more clout than little ol' me sees the same thing and interjects.

we would like to keep this thread focused around the change

Well, if you insist. What about the now odd/awkward position of having DefaultContractResolver no longer being default? Seems like there are two breaking changes here as DefaultContractResolver will have to be renamed.

MaximRouiller commented 8 years ago

What about the now odd/awkward position of having DefaultContractResolver no longer being default?

Talk to @JamesNK? or do a pull request? :stuck_out_tongue:

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

jbogard commented 8 years ago

Well, I'm at least happy that I won't have to configure all of my projects to use a community-friendly JSON style. The looks I got from front-end devs the first time they saw my PascalCase JSON, yeesh.

benaadams commented 8 years ago

If you are on the web then camelCase is the accepted standard; if you are in a closed system you can change the default, can also happily use the xml serialisation if that's your thing.

If you are not on the web generally interconnected systems are now no longer homogeneous and written in one language or on one O/S; so you don't want .NET backed apis to standout as the incongruous ones.

If you are exposing your apis publicly, or hope to do one day, you definitely want camelCase as you can't and shouldn't determine your customer's choice of language or development tools - unless you want less customers - so I applaud this change.

joseftw commented 8 years ago

In every project I ever worked in, all frontend devs has DEMANDED camelcased JSON, so I really like this decision, great work.

dotnetchris commented 8 years ago

@Eilon > Given that we are not changing the behavior, we would like to keep this thread focused around the change, as opposed to whether it's a good change or not.

So where do we take up our issues with the how this change was done out of the blue and immediately before "RTM"? Or were you directly telling us to shut up?

Eilon commented 8 years ago

@dotnetchris the change was discussed starting on March 14th, which is nearly 3 months ago now: https://github.com/aspnet/Mvc/issues/4283, and got near-universal support from the community (over 100 upvotes, which is more than any other issue in all of 'aspnet').

Then, 19 days ago, I mentioned that we decided to take this change.

Then, earlier in this thread I gave further reasoning for why we did this change.

This thread as well also shows that people are overwhelmingly in favor of this change.

We know we can't make everyone happy, but this change clearly makes more people happy than sad.

Thanks, Eilon

Mike-E-angelo commented 8 years ago

I appreciate your time in taking a moment to express the reasoning behind this, @Eilon. It really means a lot.

My primary concern here, and I will simply say this and shut up (well OK, probably not. :grin: ) is that this change affects much more than this repo/product/project. Of course it is going to have high marks here, (much like project.json did/does), but externally, it might not do so well. As long as you understand that and you have heard me, then I can feel better about accepting this. :)

MaximRouiller commented 8 years ago

@Eilon You know what makes me happy? Clicking on all the positive reactions below your comments.

It will finally make it easier to integrate with AngularJS/KnockoutJs/[insert other JS framework] in the future. Thanks for making this decision.

MaximRouiller commented 8 years ago

@Mike-EEE After double checking, this will not impact you unless you are parsing JSON by hand.

If you are using JSON.NET to generate/parse your data? No impact what so ever unless you are serializing Dictionaries. Assuming that behavior hasn't been fixed already.

Source

Mike-E-angelo commented 8 years ago

@MaximRouiller Can you please explain what you mean by parsing by hand? As in it will only affect my files if I am editing them and not using the serializer to create them?

And no, I don't use JSON. I avoid it as much as humanly possible. I am all Xaml all the time 100%. :) Again, to me this is the principle of the matter, and I am also trying to speak for enterprise/native developers as well. Guess they will just have to chime in at a later date. :open_mouth:

dotnetchris commented 8 years ago

@Eilon so the first time i heard of this was when the aspnet announcement appeared in my git inbox. How would i have been made of aware of this sooner that I could have actually provided feedback prior to the final decision being made? It doesn't seem like anyone in this thread expected this decision and that everyone was surprised. There's clearly a gap here.

benaadams commented 8 years ago

@Mike-EEE this is about the property casing used specifically for the JSON serialization format used over http/https when used via Web Api.

You can also use XML as a serialization format among others and that will be unaffected by this change.

If you are not consuming web api REST endpoints, you will also be unaffected by this change.

MaximRouiller commented 8 years ago

@dotnetchris

@Eilon mentioned that it was talked about 3 months ago.

If you want to get part of the discussion, you can subscribe to this repository and get in on the discussions!

Mike-E-angelo commented 8 years ago

@benaadams and @MaximRouiller (and really everyone haha) OK I am with you now. Thank you for your patience and great dialogue!

Eilon commented 8 years ago

@dotnetchris all discussions related to the MVC product tend to happen right here, in the MVC repo. The Announcements repo is for things that have happened, whereas the individual "product" repos are for work we are planning and debating.

dotnetchris commented 8 years ago

@Eilon what's the purpose of these discussion threads at all if everything is a done deal and the ship has already sailed? the way i originally understood the purpose of the announcements repo was to put a much more curated feed out. It seems largely, if not entirely, pointless given this thread.

What purpose does this thread at all serve? What is there to discuss if announcements mean "this is it"?

The announcements issues might as well say "these decisions were made, you missed your chance to provide feedback, no discussion"

Also @Eilon please make sure you point a Micrsoft PR / Evangelist or whatever is the correct responsibilities at this thread. The current process of the announcements seems broken by design.

Eilon commented 8 years ago

@dotnetchris as I mentioned several times now, this change was discussed at length for nearly three months before making the change, and for a few weeks once we committed to making the change, and only then did we actually make the change. I'm not sure what you're asking us to do differently in the future.

I'm sorry this change doesn't sit well with you, but I feel that we followed an open process for making this change: we listened to what customers asked for, over one hundred people were in favor of it, almost no one was against it, and so we made the change. We discussed the change with @JamesNK , the author of Json.NET, and a well-recognized figure in the community, and even got a new feature in Json.NET that would make this a safe change to make. And as we see in this thread, it seems to me that most people also agree that this change had sufficient visibility and awareness prior to it being made.

We encourage open participation in this project by monitoring the specific product repos (MVC, EF, Razor, Kestrel) and adding comments and voting on issues.

Mike-E-angelo commented 8 years ago

We understand the open process @Eilon (now :smile: )... but it does seem that creating a discussion thread after the fact is a little late in the process (or really, not very valuable at all) -- as there really isn't anything to "discuss" outside of patting ourselves on the back. I think what @dotnetchris is suggesting is perhaps tweaking the process/workflow here so that the community can provide more/better input sooner.

It's a tricky problem, as I personally do not subscribe to these repos (there are a LOT of notifications, as you well know). However, I do subscribe like a monster to RSS feeds to pretty much every MSFT blog there is, so that is how I get my information. If there was an announcement to discuss a breaking change on a blog post I would see it, FWIW.

It would be even better if GitHub provided an alert or "critical notification" that also allowed us to know when super important issues are going down, as well.

MaximRouiller commented 8 years ago

@Mike-EEE Currently, it's everything or nothing.

If you watch a repository, you'll be informed of every event on any issues.

This is a GitHub issue. By experience, blog posts are just like "announcement" but with less detail than what we find on GitHub.

Mike-E-angelo commented 8 years ago

Yeah I hear you @MaximRouiller, just expressing some thoughts. Not saying I have the answers, just suggestions. I am curious on how many notifications this repo sends out a day. I am already subscribed to a few and it's quite chatty. Basically goes straight to a outlook rule. :stuck_out_tongue:

MaximRouiller commented 8 years ago

I am personally only watching Announcements.

I do not have enough time to participate in design discussions. I believe that the .NET Team will deliver a workable product that will build upon standard, common practice and good faith.

So when something as simple as a default is changed, I don't blink. Not until we hit RTM and they deliver 1.0.1.

Mike-E-angelo commented 8 years ago

Yeah same... I do have Announcements watched, which is what landed me here. The suggestion/ask/desire here is to get a "Pre-announcement" mechanism where we can somehow join the conversation before it gets to this point, but I am not sure that is easy/possible ATM.

MaximRouiller commented 8 years ago

@Mike-EEE

Then you get to the problem where it's decision by committee where the committee might not even contribute (or be able to).

If you exclude design by committee, you end up with a repo that is used for asking if it breaks anyone and you don't need a repo for this. It will always break someone. In any scenario, I'll trust the team until they go crazy. Then, I'll speak. I don't have time for anything else.

khellang commented 8 years ago

Also, what qualifies for a "pre-announcement"? Sorry @Mike-EEE. Either you get involved (watch the repo and give feedback), or you don't. Simple as that. It's the same for everyone, like the hundreds of people that left feedback on the original proposal.

Mike-E-angelo commented 8 years ago

LOL geeze duders, I'm looking to help out here. The goal/ideal here is to get visibility to changes that people may or may not be interested in. If there are 100 people that like an issue, but there are really more like 500 that don't like it, this might be valuable to know. It's basically improving/increasing visibility of important topics without increasing noise to the average user. This is not asking for more cooks in the kitchen (design by committee) but more visibility which leads to more participation (think: GitHub reactions/votes rather than long exhausting threads like this one for the most part -- ideally), which leads to happier developers which leads to happier community which leads to a better product.

In theory. :smile:

MaximRouiller commented 8 years ago

@Mike-EEE

With what happened to https://github.com/aspnet/Home/issues/1433, I believe that there will be too many cooks in the kitchen. Or at least, lots of sideline coach. Each wanting to contribute, but lots of them not having a complete big picture and none doing any actual basic Proof Of Concepts or Pull Requests. And that is if they even installed the actual framework.

So let's retake the same scenario. Change of default to the JSON Serializer. What would a discussion on the subject would have look like? I'm not sure but I'm betting lots of people would have said exactly what was said here. We would either still be discussing it or it would have been approved anyway and people would have complained that Microsoft wasn't listening to comments.

tldr I'm ok with how things are.

Mike-E-angelo commented 8 years ago

I understand your concern @MaximRouiller. I hear you haha. The difference between what we have now and what we're discussing is that we would have dialogue BEFORE any changes were made, rather than after. I would feel better if:

1) I knew before I jumped into this discussion that it was "already too late" for any sort of change. 2) I knew beforehand there was some way of voicing my concerns (or more importantly in my mind: votes/reactions -- ala GitHub reactions, which at present is the greatest instrument for measuring sentiment) about this issue before it was sold and set in stone.

I'm OK with having my vote rejected, as long as it is heard. I am not OK with not having my vote utilized or counted or making it in the tally in the first place -- regardless of the reason. We can chalk it up to my ignorance (lack of tooling/notifications), or poor process, but the fact that my vote (or anyone's vote) didn't make it into the final tally is really what should be the focus here, and what I am driving at.

Again it is all about promoting awareness and participation so that as many metrics as possible are provided to the stakeholders and drivers to the repo. Hope that clarifies my position. :)

clarkis117 commented 8 years ago

@MaximRouiller @Mike-EEE Do I hear a need for a discussion on changing the default serialization format for Web APIs to Protobuf? ;-)

benaadams commented 8 years ago

@Elion I added suggestion to https://github.com/aspnet/Home/issues/1571 on a way to make breaking changes more visible while they are still at the "potential" stage

@Mike-EEE would that help?

Mike-E-angelo commented 8 years ago

@Mike-EEE would that help?

Does it ever! Thank you for pointing that out, @benaadams! And thank you, @Eilon!!!

SocVi100 commented 8 years ago

It's ok to have the serializer configurable, but, it sounds weird to be forced to tweak the configuration when you want to use the default configuration of the serializer (DefaultContractResolver). It would be far more intuitive to tweak the configuration when you want anything different than the default... wouldn't it?

MaximRouiller commented 8 years ago

@SocVi100

DefaultContractResolver is only default as far as its name. It was probably called default because it was the default way of serializing in .NET before (think DataContract) JSON.NET became popular.

Maybe we should ask @JamesNK for some history on the naming. ;-)

And yeah... DefaultContractResolver is part of JSON.NET. Not .NET.

fhelwanger commented 8 years ago

@SocVi100 DefaultContractResolver is a Json.NET class, it's not an ASP.NET Core MVC class. I think it'd be better named as PascalCaseContractResolver.

I agree with you... it's confusing.

John0King commented 8 years ago

@fhelwanger the DefaultContractResolver is not PascalCaseContractResolver , you can use DefaultContractResolver to generate camelCased json when you write camelcased C# property or use [JsonProperty("cameCaseWords")] . This is why it called default , because it don't change any thing you write . and PascalCaseContractResolver sound like it will change anything you write to PascalCase

fhelwanger commented 8 years ago

@John0King You're right, thanks!

antonGritsenko commented 8 years ago

Does some one tested the workaround? services .AddMvc() .AddJsonOptions(options => options.SerializerSettings.ContractResolver = new DefaultContractResolver());

It doesn't work in my case for some reason (.net core 1.0 RTM), still have propertyName in JSON. What it can be?

UPDATE: Found the reason, related to Toolling.

iamthemovie commented 8 years ago

@antonGritsenko that's my work around too, working fine for me.

NeelBhatt commented 8 years ago

I like this change and I also wrote a little blog so that more people can know about this change.

Blog is here.