camelCase all the JSONs #746

SteveSandersonMS · 2018-05-03T23:00:25Z

We know from feedback that C# developers pretty much universally want automatic PascalCase-camelCase conversions enabled during JSON serialization/deserialization. So with this PR, JsonUtility will now:

When serializing, convert property names to camelCase
When deserializing, match incoming property names case insensitively

Since JsonUtility is used internally for HttpClient's XyzJsonAsync methods (e.g., GetJsonAsync), it affects what data is sent, and broadens what incoming data is accepted. Likewise, since JsonUtility is used for RegisteredFunction.Invoke, it affects how complex .NET objects are passed to JS-side code.

A very important design point is that this is not overridable or configurable. We are trying very hard not to develop a whole new sophisticated JSON library inside Blazor. Our goal for the built-in JSON utility is to support the most basic and common things that are absolutely essential for core scenarios. Many developers will want something more customisable, which is fine: there are lots of other JSON libraries on NuGet (and where necessary, we'll adapt our APIs to make sure you can use them). JSON libraries should be separate, independent projects from Blazor.

Hopefully this change will satisfy the majority of developers.

Breaking change If you do sophisticated JS interop and have JS-side code that relies on property names arriving in PascalCase, you'll need to change it to expect camelCase.

rynowak · 2018-05-04T06:45:00Z

src/Microsoft.AspNetCore.Blazor/Json/CamelCase.cs

+                }
+            }
+
+            return new string(chars);


/cc @JamesNK

rynowak · 2018-05-04T06:46:23Z

src/Microsoft.AspNetCore.Blazor/Json/CamelCase.cs

+                return value;
+            }
+
+            if (!char.IsUpper(value[0]))


Are we generally just dealing with things that are valid C# identifiers? or does this need to camel case arbitrary strings?

This seems to have the assumption that the input is always already a PascalCase or camelCase string

For example, some cases I'm not really sure of the expected output:
foo_Bar
ThisIsXML

OK looks like this is addressed below.

Yes, the whole notion of camelcasing is more subjective than first appears. There are plenty of strings where people would disagree about what the expected camelcase verson is, and different libraries do produce different output in practice.

My goal in this case was, for the subset of cases we care about (incoming string is a member identifier), to produce the same result as Json.NET. That's to make it easier for anyone who wants to "grow up" to Json.NET after starting with the built-in utility, or who uses Json.NET on the server and expects their client code to work like other .NET clients.

rynowak · 2018-05-04T06:49:31Z

src/Microsoft.AspNetCore.Blazor/Json/CamelCase.cs

+{
+    internal static class CamelCase
+    {
+        public static string ToCamelCase(this string value)


Can we make this not an extension method? It's used in like 4 places 😆

OK, fair point. Will do.

rynowak · 2018-05-04T06:53:13Z

test/Microsoft.AspNetCore.Blazor.Test/JsonUtilTest.cs

@@ -143,6 +201,38 @@ public void SupportsInternalCustomSerializer()
            Assert.Equal("{\"key1\":\"value1\",\"key2\":123}", json);
        }

+        // Test cases based on https://github.com/JamesNK/Newtonsoft.Json/blob/122afba9908832bd5ac207164ee6c303bfd65cf1/Src/Newtonsoft.Json.Tests/Utilities/StringUtilsTests.cs#L41
+        // The only difference is that our logic doesn't have to handle space-separated words,
+        // because we're only use this for camelcasing .NET member names


OK so that's the answer to my question from earlier then 👍

rynowak · 2018-05-04T06:54:22Z

src/Microsoft.AspNetCore.Blazor/Json/CamelCase.cs

+                        break;
+                    }
+                }
+            }


Since the result of camel casing is cached, is there much value in doing perf optimizations here? This only runs once per member per type right?

I agree the value is limited. But it wasn't difficult to implement and doesn't add a lot of complexity. The main benefit vs the Json.NET implementation is that this version only does one IsUpper check per character rather than two. But I agree it's a super-minor benefit.

rynowak · 2018-05-04T06:54:43Z

src/Microsoft.AspNetCore.Blazor/Json/CamelCase.cs

+            if (string.IsNullOrEmpty(value))
+            {
+                return value;
+            }


Is this considered invalid input since only we're calling this, and only for valid member names?

Hmm. Debatable.

If we were to reject "", then I think we'd at least have to rename this to CamelCaseDotNetMemberName because it couldn't call itself a general camelcaser. The existing implementation, even without doing anything about spaces, I think fits a reasonable definition of a general camelcase implementation.

Since it's internal anyway I'd rather not get hung up on this. Let me know if you feel strongly about it.

OK on more consideration I will be more explicit about limiting the scope to .NET identifiers and will rename the method accordingly.

Renamed to MemberNameToCamelCase

rynowak · 2018-05-04T07:00:17Z

src/Microsoft.AspNetCore.Blazor/Json/CamelCase.cs

+            {
+                // Basic cases where only the first char needs to be modified
+                chars[0] = char.ToLowerInvariant(chars[0]);
+            }


Is there much reason to have this special case? It looks to me like this would immediately hit the 'break' condition in the loop.

Ah yes, I see it's not clear from the comment but this branch is functionally required. Without it, the logic would never look at the value of chars[1] and would produce an incorrect result for inputs like OnCLICK (it would produce onclick instead of onCLICK).

Basically chars[0] and chars[1] are both special, because the "when to stop" logic is based on looking at the n+2 character versus the one we're currently lowercasing. Since the loop starts at index 0 that means it only starts looking at the case of characters from 2 onwards.

I'll update the comment to clarify this.

rynowak

I left some minor comments here but overall this seems sound to me.

…retain case on dictionary keys)

…eird otherwise

* Add camelCase utility * Use camelCase when JSON-serializing (but not for dictionary keys) * Make JSON deserialization treat member names case-insensitively (but retain case on dictionary keys) * Use camelCase in JSON in the samples and templates * Reverse the order of the params for the camelcase test because it's weird otherwise * CR feedback

SteveSandersonMS requested review from javiercn and rynowak May 3, 2018 23:00

SteveSandersonMS mentioned this pull request May 3, 2018

SimpleJson Add support for camelCase json to .NET and vice versa #644

Closed

SteveSandersonMS added this to the 0.4.0 milestone May 3, 2018

SteveSandersonMS added the enhancement label May 3, 2018

rynowak reviewed May 4, 2018

View reviewed changes

src/Microsoft.AspNetCore.Blazor/Json/CamelCase.cs

}

}

return new string(chars);

Copy link

Member

rynowak May 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @JamesNK

rynowak reviewed May 4, 2018

View reviewed changes

rynowak approved these changes May 4, 2018

View reviewed changes

SteveSandersonMS added 6 commits May 4, 2018 15:15

Add camelCase utility

0bec15a

Use camelCase when JSON-serializing (but not for dictionary keys)

aaa3f00

Make JSON deserialization treat member names case-insensitively (but …

7a2098a

…retain case on dictionary keys)

Use camelCase in JSON in the samples and templates

56f5c6d

Reverse the order of the params for the camelcase test because it's w…

a299748

…eird otherwise

CR feedback

bfce965

SteveSandersonMS force-pushed the stevesa/json-camelcase branch from d247ed4 to bfce965 Compare May 4, 2018 14:37

SteveSandersonMS merged commit 47169ab into dev May 4, 2018

SteveSandersonMS deleted the stevesa/json-camelcase branch May 4, 2018 15:14

StevenTCramer mentioned this pull request May 14, 2018

Upgrade to Blazor 0.4.0 .net sdk 2.1.0 and fix camelCase issue. BlazorExtensions/Logging#6

Merged

danroth27 added the 3 - Done label Jun 7, 2018

chanan mentioned this pull request Jul 15, 2018

Getting IndexOutOfRange exception when initializing BlazorDB chanan/BlazorDB#12

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

camelCase all the JSONs #746

camelCase all the JSONs #746

SteveSandersonMS commented May 3, 2018 •

edited

Loading

rynowak May 4, 2018

rynowak May 4, 2018

rynowak May 4, 2018 •

edited

Loading

rynowak May 4, 2018

SteveSandersonMS May 4, 2018

rynowak May 4, 2018

SteveSandersonMS May 4, 2018

SteveSandersonMS May 4, 2018

rynowak May 4, 2018

rynowak May 4, 2018

SteveSandersonMS May 4, 2018

rynowak May 4, 2018

SteveSandersonMS May 4, 2018

SteveSandersonMS May 4, 2018

SteveSandersonMS May 4, 2018

rynowak May 4, 2018

SteveSandersonMS May 4, 2018

SteveSandersonMS May 4, 2018

rynowak left a comment

camelCase all the JSONs #746

camelCase all the JSONs #746

Conversation

SteveSandersonMS commented May 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rynowak May 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rynowak left a comment

Choose a reason for hiding this comment

SteveSandersonMS commented May 3, 2018 •

edited

Loading

rynowak May 4, 2018 •

edited

Loading