change snake_case to camelCase fields name #89
Conversation
| @@ -1933,6 +1933,15 @@ function createMessage( | |||
| pbIdentifier | |||
| ) { | |||
| const members = []; | |||
| messageDescriptor.field = messageDescriptor.field.map(f => { | |||
There was a problem hiding this comment.
IMHO - this shouldn't be the default behavior, but rather an parameter to the generator to enable the behavior of camelCasing.
There was a problem hiding this comment.
I think snake_case should be possible through param but default behavior must be camelCase. I can't see any styleguide/naming convention in Typescript that publicise snake_case as variable naming. Here is the one from Google where gRPC was born https://google.github.io/styleguide/tsguide.html
There was a problem hiding this comment.
Wouldn’t this be an opinion/preference that we enforce on users? (which I don’t like tbh).
I would argue that default behavior should be camelCase. I strongly believe that we should use whatever is there (current behavior) and only change it if an option such as --ts_opt=casing=camel is present.
|
Grpc-js (the official implementation) convert to camel case. It’s the
normal behavior in many languages
…On Tue 5 Oct 2021 at 01:58, David Koblas ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/descriptor.js
<#89 (comment)>:
> @@ -1933,6 +1933,15 @@ function createMessage(
pbIdentifier
) {
const members = [];
+ messageDescriptor.field = messageDescriptor.field.map(f => {
IMHO - this shouldn't be the default behavior, but rather an parameter to
the generator to enable the behavior of camelCasing.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#89 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACRE5P3CF2D5FTOZMWZ6S3UFI5UBANCNFSM5EW3TCZA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
Hi @thesayyn, can you help us make this go ahead ? Some test are failing due to case changes. |
|
Hey. i will take a look at this. |
|
Looks like there are two pull requests open doing the same thing, which seems to suggest there is a strong desire to have camel case support. Is there a path forward on releasing this? I'd be unfortunate for users to have to make more forks just because we are not merging any PRs. |
It is not that I am not willing to merge these PRs. As you can see that they are failing the existing tests yet they lack tests for this new behavior. Unfortunately, I don’t have time to pick up every issue and fix it myself. hence I would really appreciate a proper PR (with tests) introducing this new behavior. |
|
closing this in favor of #98 |
As discuss in the #78 issue, protocol buffer use snake_case for their field name, however the use of camelCase is more appropriate in JS/TS and more generally in generated code.