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

grpc-js generated client constructor missing some allowed options #693

Closed
thomasboyt opened this issue Oct 24, 2022 · 2 comments · Fixed by #704
Closed

grpc-js generated client constructor missing some allowed options #693

thomasboyt opened this issue Oct 24, 2022 · 2 comments · Fixed by #704
Labels

Comments

@thomasboyt
Copy link

Small type generation bug (I hope to contribute a fix, but wanted to make sure to open an issue in case I forget!). For grpc-js generated clients, the following constructor is used:

new (
  address: string,
  credentials: ${ChannelCredentials},
  options?: Partial<${ChannelOptions}>,
): ${serviceDesc.name}Client;

However, options here is not correct: it should use the ClientOptions type (exported from grpc-js) instead, which is Partial<ChannelOptions> plus some extra fields that you can pass to a client constructor (see https://grpc.github.io/grpc/node/grpc.Client.html).

Very minor bug in the grand scheme of things and easy to work around since it's just a type bug, but I think an easy fix unless I'm missing something.

@stephenh
Copy link
Owner

@thomasboyt hey! Sorry for the late reply, I can only triage ts-proto issues every other week or so...

It'd great if you could submit a PR for this!

I think the fix would be around here:

https://github.com/stephenh/ts-proto/blob/main/src/generate-grpc-js.ts#L239

Thanks!

@stephenh
Copy link
Owner

🎉 This issue has been resolved in version 1.132.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants