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

Add a way to pass loader options to custom grpc-loader #13930

Open
1 task done
eladhaim opened this issue Aug 29, 2024 · 9 comments
Open
1 task done

Add a way to pass loader options to custom grpc-loader #13930

eladhaim opened this issue Aug 29, 2024 · 9 comments
Labels
needs triage This issue has not been looked into type: enhancement 🐺

Comments

@eladhaim
Copy link

eladhaim commented Aug 29, 2024

Is there an existing issue that is already proposing this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe it

We heavily rely on protobuf and use ts-proto to generate our package definitions. To handle the generated code, we've had to create our own gRPC loader. However, while we have the option to pass a custom gRPC loader, we currently cannot pass custom gRPC options. Ideally, we'd like the loader object within GrpcOptions to be of type any or a generic type that we can override to customize the options as needed.

Describe the solution you'd like

Our problem is just a type issue, converting the loader object to any/generic with default will do the job.

Teachability, documentation, adoption, migration strategy

Today:

     ClientsModule.register([
      {
        name: 'GRPC_CLIENT_NAME',
        transport: Transport.GRPC,
        options: {
          package: 'testPackage',
          protoPath: 'proto_path',
          protoLoader: 'our custom loader',
          url: 'localhost:4001',
          loader: {
             // @ts-ignore
             newOption: 'new option'
          },
        },
      }
    ]),

After the change:

   ClientsModule.register([
   {
     name: 'GRPC_CLIENT_NAME',
     transport: Transport.GRPC,
     options: {
       package: 'testPackage',
       protoPath: 'proto_path',
       protoLoader: 'our custom loader',
       url: 'localhost:4001',
       loader: {
          newOption: 'new option'
       },
     },
   }
 ])

What is the motivation / use case for changing the behavior?

Adding more flexibility to the server/client of grpc

@eladhaim eladhaim added needs triage This issue has not been looked into type: enhancement 🐺 labels Aug 29, 2024
@wrspada02
Copy link

Do you think would be nice pass some kind of generic type to the loader object instead apply type any? Just to keep the type safe. Also, I'll figure it out and try to implement this.

@eladhaim
Copy link
Author

yes sure i think generic type will be great, i can submit a pr if needed.

@wrspada02
Copy link

May I try to implement this? It'd be my first contribuition on the open-source world.

@eladhaim
Copy link
Author

go ahead, i think it is a good issue to start with

@wrspada02
Copy link

Thank you. Any updates I'll let you know.

@wrspada02
Copy link

wrspada02 commented Aug 29, 2024

Well, I think I did what you meant. I ain't able to push a PR on this repository, however, I just did a fork and pushed it there. Could you check it out and confirm if makes sense? If so, we can request to open a PR and push this commit into this repository.

Commit link - wrspada02@dc3d1af

Thanks in advance.

image

@eladhaim
Copy link
Author

eladhaim commented Sep 1, 2024

👍

@wrspada02
Copy link

wrspada02 commented Sep 1, 2024

Hello someone at @nestjs. May we create a PR for this type feature addition? I ain't able to open a PR for this repository.

@CaioF
Copy link

CaioF commented Sep 8, 2024

@eladhaim quick question, is your custom loader able to serialize/deserialize google.protobuf types when generating with ts-proto? Because I have an issue where nestJS seems to ignore the wrappers from ts-proto and therefore I cannot use any of the well known types. stephenh/ts-proto#1106 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs triage This issue has not been looked into type: enhancement 🐺
Projects
None yet
Development

No branches or pull requests

3 participants