Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Camelcase is lost (Javascript protoc) #17

Open
zim32 opened this issue May 12, 2021 · 8 comments
Open

Camelcase is lost (Javascript protoc) #17

zim32 opened this issue May 12, 2021 · 8 comments
Assignees
@lukesandberg
Labels
bug Something isn't working good first issue Good for newcomers javascript protoc triaged Issue has been triaged

Comments

@zim32
Copy link

zim32 commented May 12, 2021

What version of protobuf and what language are you using?
3.16.0 (Linux x86_64)
Javascript

Operation system
Ubuntu 18.04

What runtime / compiler are you using (e.g., python version or gcc version)

What did you do?
Create proto file (syntax 3)

syntax = "proto3";

message Test {
  string someFieldWithCamelCase = 1;
}

run

protoc --js_out=import_style=commonjs,binary:out test.proto

Got

proto.Test.prototype.setSomefieldwithcamelcase = function(value) {
  return jspb.Message.setProto3StringField(this, 1, value);
};

What did you expect to see

proto.Test.prototype.setSomeFieldWithCamelCase = function(value) {
  return jspb.Message.setProto3StringField(this, 1, value);
};

What did you see instead?
Camelcase is lost
@k10-patil
Copy link

k10-patil commented Jun 17, 2021 •
edited

any updates on this? facing same issue. It would be nightmare to edit fieldnames when dealing with large number of proto messages!

@petermetz
Copy link

petermetz commented Aug 11, 2021 •
edited

Having the same problem, just came here to gently bump the thread. :-)

Additional testing also revealed that if I load the proto files directly with grpc-loader[1], then the dynamically generated JS code has the correct (camel) casing.

[1]:

import * as grpc from "@grpc/grpc-js";
import * as protoLoader from "@grpc/proto-loader";

@petermetz petermetz mentioned this issue Aug 11, 2021
Closed
@trent1000
Copy link

trent1000 commented Aug 11, 2021 via email

petermetz referenced this issue in petermetz/cacti Aug 11, 2021
@petermetz
feat(cmd-api-server): support grpc web services hyperledger#1189
0638005 
TODO:
1. Figure out how to stop the generator from mangling the model
property names into capital case (getCreatedat)
See: https://github.com/protocolbuffers/protobuf/issues/8608

2. Figure out how to export the OpenAPI model classes and the
gRPC model classes at the same time without name conflicts
(they have the same names)

3. Implement streaming healthcheck endpiont with gRPC

4. Allow plugins to hook in their own gRPC service implementations.

5. Verify with test case that the secure credentials flavor also
works.

Fixes hyperledger#1189

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@petermetz petermetz mentioned this issue Aug 11, 2021
Merged
petermetz referenced this issue in petermetz/cacti Aug 11, 2021
@petermetz
feat(cmd-api-server): support grpc web services hyperledger#1189
3df47d2 
Primary change:
--------------
The API server now contains a gRPC server in addition to what it
had before (HTTP+SocketIO servers) so that through this it can
provide the opportunity for plugins to expose their gRPC services
via the Cactus API server. It is possible for plugins to serve
their requests on multiple protocols at the same time which means
that this is a big step towards our goal of being properly multi-
protocol capable.

Secondary change(s):
-------------------
1. The "allowJs" flag for the Typescript compiler is now specified
as true for the cmd-api-server package which is necessary because
without this the .js files generated by protoc do not get copied
over from the ./src/main/typescript/generated/ folder to the
./dist/lib/.../generated folder with the rest of the build files.
It is not ideal that we are generating JS code into the TS folder
but it does come with .d.ts files and this is the "state of the
art" at the moment becaues we can only do what the protocol
buffer compiler will support and this is it.

Later on we should improve on this situation once the tooling
catches up and adds support to generate straight up TS code instead
of JS+d.ts files so that we keep to our convention of having only
TS code under ./src/main/typescript for obvious reasons.

TODO:
----

1. Figure out how to stop the generator from mangling the model
property names into capital case (getCreatedat)
See: https://github.com/protocolbuffers/protobuf/issues/8608

2. Figure out how to export the OpenAPI model classes and the
gRPC model classes at the same time without name conflicts
(they have the same names)

3. Implement streaming healthcheck endpiont with gRPC

4. Allow plugins to hook in their own gRPC service implementations.

5. Verify with test case that the secure credentials flavor also
works.

Fixes hyperledger#1189

Signed-off-by: Peter Somogyvari <peter.somogyvari@accenture.com>
@youurayy
Copy link

This is an absolute deal-breaker of using google-protobuf in javascript.

@thesayyn
Copy link

thesayyn commented Dec 8, 2021

I don’t think that google will change this behaviour anytime soon.

also seen on #37

@nnnnoel nnnnoel mentioned this issue Apr 12, 2022
Closed
@nnnnoel nnnnoel mentioned this issue Apr 22, 2022
Closed
@acozzette acozzette transferred this issue from protocolbuffers/protobuf May 16, 2022
@dibenede dibenede added bug Something isn't working good first issue Good for newcomers triaged Issue has been triaged labels Sep 2, 2022
@dibenede
Copy link
Contributor

dibenede commented Sep 2, 2022

This is fixable and is inconsistent with other proto runtimes. However, it poses a compatibility problem. The issue is here:

protobuf-javascript/generator/js_generator.cc

Line 290 in a428c58

running += ToLowerASCII(input[i]);

The immediate workaround is to use underscore_naming style rather than camelCase.

The fix would be to flag gate the naming behavior. We'll need to consider which direction the flagging should operate.

@sarahec
Copy link

sarahec commented Oct 23, 2022 •
edited

My suggestion: the default behavior should match the style guide (which calls for snake_case in field names, and CamelCase for the proto name), and have a flag to enable the extension (or extensions).

If the flag's not set, this could emit a warning on seeing a CamelCase field name.

P.S. I'm willing to take on this patch once we have agreement on how it should work.

zhekai-jiang added a commit to epfldata/sudokube that referenced this issue May 10, 2023
@zhekai-jiang
Change field names to lower_snake_case
cb32082 
According to protobuf's guidelines: https://protobuf.dev/programming-guides/style/#message-field-names

And because camelCase will be lost and become all lowercase in the generated TypeScript/JavaScript code: protocolbuffers/protobuf-javascript#17
zhekai-jiang added a commit to zhekai-jiang/sudokube-webui that referenced this issue May 10, 2023
@zhekai-jiang
Change field names to lower_snake_case
a8ac66d 
According to protobuf's guidelines: https://protobuf.dev/programming-guides/style/#message-field-names

And because camelCase will be lost and become all lowercase in the generated TypeScript/JavaScript code: protocolbuffers/protobuf-javascript#17
zhekai-jiang added a commit to epfldata/sudokube that referenced this issue Jul 12, 2023
@zhekai-jiang
WebUI: Change field names to lower_snake_case
379b0a0 
According to protobuf's guidelines: https://protobuf.dev/programming-guides/style/#message-field-names

And because camelCase will be lost and become all lowercase in the generated TypeScript/JavaScript code: protocolbuffers/protobuf-javascript#17
@jmzwcn
Copy link

jmzwcn commented Oct 25, 2023 •
edited 

syntax = "proto3";

message Test {
  string some_field_with_camel_case = 1;
}

https://protobuf.dev/programming-guides/style/#message-field-names

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lukesandberg lukesandberg

Labels
bug Something isn't working good first issue Good for newcomers javascript protoc triaged Issue has been triaged
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests