ngx-grpc icon indicating copy to clipboard operation
ngx-grpc copied to clipboard

Package name with uppercase letters is not properly generated

Open esoubiran-aneo opened this issue 3 years ago • 7 comments

Let's assume the following proto file:

syntax = "proto3";

package ArmoniK.api.grpc.v1.submitter;

service Submitter{
	rpc CancelSession (Session) returns (Empty);
}

message Session {
	string id = 1;
}

message Empty {}

The generator will not work because it wil generate this:

Observable<GrpcEvent<thisProto.ArmoniK.api.grpc.v1.submitter.Empty>>

instead of this

Observable<GrpcEvent<thisProto.Empty>>

esoubiran-aneo avatar Jul 22 '22 09:07 esoubiran-aneo

hi @esoubiran-aneo

could you please share the version of protoc?

smnbbrv avatar Jul 22 '22 09:07 smnbbrv

I think it's in this package ? @smnbbrv

"grpc-tools": "^1.11.2",
"@ngx-grpc/protoc-gen-ng": "^3.0.1"

esoubiran-aneo avatar Jul 22 '22 09:07 esoubiran-aneo

But I use Windows ! I will try the linux command to see if there is a difference

esoubiran-aneo avatar Jul 22 '22 10:07 esoubiran-aneo

Yes, this is using protoc 3.6.1 on Ubuntu (libprotoc 3.6.1)

cancelSession: (
    requestData: thisProto.ArmoniK.api.grpc.v1.Session,
    requestMetadata = new GrpcMetadata()
  ): Observable<GrpcEvent<thisProto.ArmoniK.api.grpc.v1.Empty>> => {
    return this.handler.handle({
      type: GrpcCallType.unary,
      client: this.client,
      path: '/ArmoniK.api.grpc.v1.Submitter/CancelSession',
      requestData,
      requestMetadata,
      requestClass: thisProto.ArmoniK.api.grpc.v1.Session,
      responseClass: thisProto.ArmoniK.api.grpc.v1.Empty
    });
  }

(ArmoniK.api.grpc.v1 is not defined)

with this package: package ArmoniK.api.grpc.v1;

esoubiran-aneo avatar Jul 22 '22 10:07 esoubiran-aneo

I guess this is an issue and it should be fixed on a lib side. However, according to the google protobuf styleguide

Package names should be in lowercase. Package names should have unique names based on the project name, and possibly based on the path of the file containing the protocol buffer type definitions.

Please use the lowercase names only for packages and the issue won't appear.

smnbbrv avatar Jul 22 '22 10:07 smnbbrv

You're a king man! It seems to work now that package name is "armonik.api.grpc.v1"!

esoubiran-aneo avatar Jul 22 '22 10:07 esoubiran-aneo

you are welcome! I'm glad it worked out!

Please leave the issue open, when there is time to deal with it, it's gonna be fixed

smnbbrv avatar Jul 22 '22 11:07 smnbbrv