protobuf.js icon indicating copy to clipboard operation
protobuf.js copied to clipboard

feat: honor json_name option

Open michaelbgreen opened this issue 2 years ago • 11 comments

Honor the json_name option. This is useful when a .proto file is compiled for multiple languages, to conform to whatever random casing rules were chosen for the project:

  • The .proto convention is to use lower_snake_case for field names, and UPPER_SNAKE_CASE for enum names
  • C++ (via protoc) simply uses lower_snake_case for field names
  • C# (via protobuf-net) uses PascalCase for both field and enum names, and has options for customizing the names
  • JavaScript (via protobuf.js) uses camelCase for field names, and the json_name attribute can now be used to customize the name

Example:

message NetworkAdapter {
  string get_ip_address = 1 [json_name = "getIPAddress", (.protobuf_net.fieldopt).name ="GetIPAddress"];
}

Default names without the options:

  • C++: get_ip_address (unchanged)
  • C#: GetIpAddress
  • JS: getIpAddress

Fixes #992, fixes #1245, fixes #1775

michaelbgreen avatar Oct 12 '22 18:10 michaelbgreen

Can we prioritize this review? We need it as well.

aneeshgarg avatar Dec 19 '22 21:12 aneeshgarg

Will this change also use json_name for files generated statically?

aneeshgarg avatar Dec 19 '22 21:12 aneeshgarg

Any update on this? Would really like to see this land.

tomquist avatar Mar 26 '23 11:03 tomquist

it seems we have a solution here for this https://github.com/protobufjs/protobuf.js/issues/564#issuecomment-280923387

YashJainSC avatar Aug 03 '23 07:08 YashJainSC

The linked comment is not exactly a solution as it doesn't respect the explicitly defined json_name.

tomquist avatar Nov 24 '23 07:11 tomquist

Is there any way I could help this PR move forward and get merged?

I just did a quick test against v7.2.5 and it seems to be working. I really need this functionality and I would like to to avoid having to maintain my own fork.

mbohal avatar Nov 28 '23 16:11 mbohal

+1 to releasing it as a major version. This change will definitely break my current use case.

Can this be behind a build flag/option?

aneeshgarg avatar Dec 09 '23 00:12 aneeshgarg

Alternatively this could be a runtime option which is off by default.

tomquist avatar Dec 09 '23 05:12 tomquist

Runtime option might not work with folks like us using the statically generated files.

aneeshgarg avatar Dec 09 '23 05:12 aneeshgarg

I think it can technically go to IParseOptions (right near the keepCase) and to the CLI flag of pbjs for static proto generation. That way we can make it false by default and avoid making a semver major version. Not sure if @michaelbgreen has time to implement it that way though :) (I might have but later this month).

alexander-fenster avatar Dec 11 '23 16:12 alexander-fenster

I am happy to see that this is getting traction, but sorry, I no longer have time to contribute to this.

michaelbgreen avatar Dec 12 '23 14:12 michaelbgreen