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.

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

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
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

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

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";

@trent1000
Copy link

trent1000 commented Aug 11, 2021 via email

petermetz referenced this issue in petermetz/cacti Aug 11, 2021
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-cacti#1189

Signed-off-by: Peter Somogyvari <[email protected]>
petermetz referenced this issue in petermetz/cacti Aug 11, 2021
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-cacti#1189

Signed-off-by: Peter Somogyvari <[email protected]>
@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

@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:

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

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
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
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
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

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
Labels
bug Something isn't working good first issue Good for newcomers javascript protoc triaged Issue has been triaged
Projects
None yet
Development

No branches or pull requests