nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
67.29k stars 7.59k forks source link

gRPC Client does not send fields when field name contains underscore #4917

Closed RobertB93 closed 4 years ago

RobertB93 commented 4 years ago

Bug Report

Current behavior

Having a Nestjs gRPC Client using a .proto file that contains message field names with an underscore leads to those fields not being sent and thus falling back to their defaults.

Input Code

Does not work:

message ConfigNew {
    int32 id = 1;
    string test_string = 2;
}

works:

message ConfigNew {
    int32 id = 1;
    string teststring = 2;
}

Expected behavior

Support for message field names, regardless whether they contain an underscore or not.

Environment


- Nest version: 7.2.0
- Node version: v12.16.2
- Platform: Linux
kamilmysliwiec commented 4 years ago

Please provide a minimum reproduction repository.

miladabc commented 4 years ago

set keepCase option to true in microservice options.
I think this option should be true by default, it's not at the moment.
Like this:

app.connectMicroservice<MicroserviceOptions>({
    transport: Transport.GRPC,
    options: {
      package: 'test',
      protoPath: 'path/to/proto',
      loader: { keepCase: true },
    },
  });
RobertB93 commented 4 years ago

set keepCase option to true in microservice options. I think this option should be true by default, it's not at the moment. Like this:

app.connectMicroservice<MicroserviceOptions>({
    transport: Transport.GRPC,
    options: {
      package: 'test',
      protoPath: 'path/to/proto',
      loader: { keepCase: true },
    },
  });

Yes, you are right! It works as expected when setting the loader options, thank you. I think we should either enable this by default (if applicable) or at least mention in the docs, what do you guys think?

kamilmysliwiec commented 4 years ago

Let's track this here https://github.com/nestjs/docs.nestjs.com/issues/1297

maurolacy commented 3 years ago

I think that yes, you should set this to true by default, and yes, you should mention this in the docs. This is a stupid "feature". GRPC is intended for inter-systems communication. That potentially means or includes different operating systems, different programming languages, and, yes, different case conventions too.