version 1.90.0 broke snakeToCamel=false functionality

[SchroederChris]

Hey, as you already discussed in #408, that PR introduced breaking changes when using snakeToCamel=false.

In our case, the json we're receiving contains snake_case properties, so all fromJSON methods accessing those properties are broken with the new version.

I'd very much appreciate getting back the old behavior as you suggested with the keys-and-json option.

[stephenh]

@SchroederChris thanks for reporting.

@ajaykarthikr or @SchroederChris , could either of you submit a PR for this?

I think changing the snakeToCamel: boolean to snakeToCamel: boolean | string and then supporting snakeToCamel=keys,json where if it's not "true" or "false" we split on ","?

So to the rest of the code generator, snakeToCamel would look like snakeToCamel: Array<"keys" | "json">, but we support the user setting true / false and then map those to true --> ["keys"] and false --> [] and then keys,json => ["keys", "json"].

[ajaykarthikr]

Ohhw, I think I got your general idea. But can you please clarify one thing? When there's json in snakeToCamel flag, should the json field names be camelCase or snake_case

[stephenh]

Oh right, snakeToCamel "with json in it" insinuates json would be camelized.

How about:

• snakeToCamel=true (the default) --> ["keys", "json"]
• snakeToCamel=false --> []
• snakeToCamel=keys --> ["keys"], use camel cased message keys, but still snake case in to/from JSON
• snakeToCamel=json --> ["json"], use proto3-spec camel casing in to/fromJSON, but still snake case in message keys
• snakeToCamel=keys,json --> ["keys","json"]

I think the ^ would actually restore snakeToCamel=false to the previous behavior, which is that it meant neither message keys or json fields were camelized.

But, if a user wanted, they could use snake keys (i.e. Message.first_name) but json fields (so message: { firstName: ... } }), which is not something the can do today with just true / false.

And then whenever config.snakeToCamel.includes("json") we would use the jsonName from protobuf (which they've camelized for us), but if "json" is not in the list, we'd use the field name, which would restore the behavior @SchroederChris wants.

Does that sound good?

[SchroederChris]

@stephenh I like that solution 👍 It allows for full flexibility and still keeps backwards compatibility.

@ajaykarthikr thank you for taking care of this! You're probably more familiar with it than me atm. as you just changed it recently while for me it's been several months at least 😃

[ajaykarthikr]

@stephenh Thanks for the detailed explanation. I add a PR soon.

Can you guys give me a day or two to fix this? 😅

[SchroederChris]

@ajaykarthikr sure, take your time 🙂 For me this is not urgent at all - the older versions currently work just fine for us.

[stephenh]

🎉 This issue has been resolved in version 1.93.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[SchroederChris]

@ajaykarthikr @stephenh thanks guys!