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

Consider @grpc/grpc-js #2

Closed
tatemz opened this issue Oct 21, 2020 · 5 comments · Fixed by #3
Closed

Consider @grpc/grpc-js #2

tatemz opened this issue Oct 21, 2020 · 5 comments · Fixed by #3

Comments

@tatemz
Copy link
Contributor

tatemz commented Oct 21, 2020

Hey. This is a cool library!

I was recently trying to integrate this into ardatan/graphql-mesh#1086 and had some conflicts when passing @grpc/grpc-js CredentialsChannel to your Client constructor. See the relevant comment in ardatan/graphql-mesh#1086

There is a conflict when using grpc-reflection-js. The CredentialsChannel type used by it is different than the CredentialsChannel type used by @grpc/grpc-js. This is why I swapped in grpc. However, on the grpc package README, it states that this package is deprecated (presumably in favor of @grpc/grpc-node? 🤔 🤷‍♂️)
I find this odd, because when using the JS protoc plugin, all of the generated gRPC server/client code uses the grpc package.
I suppose until protoc updates it's output dependency, then using grpc should be ok. Alternatively, we can submit a PR to grpc-reflection-js to update that dependency to use @grpc/grpc-js. Whew 😅

@tatemz tatemz changed the title Consider @grpc/grpc-js Consider @grpc/grpc-js Oct 21, 2020
@redhoyasa
Copy link
Owner

redhoyasa commented Oct 26, 2020

Hey, thanks for your interest.

I have reviewed your changes. Originally, I was using grpc because I want to create some changes to uw-labs/bloomrpc.

I think we can migrate to @grpc/grpc-js since protoc also supports @grpc/grpc-js (see this for the detail).

But I will not available in a few weeks. If you're in hurry, don't hesitate to create PR for this.

@redhoyasa
Copy link
Owner

Hey @tatemz, I just published your changes in version 0.0.7

@tatemz
Copy link
Contributor Author

tatemz commented Oct 29, 2020

💯 Awesome!

Are the release/tagged versions in git synchronized with the versions in the NPM registry?

@redhoyasa
Copy link
Owner

Not yet, I was getting problems when publishing changes to NPM registry via Github Action and ended up making many unused tags and releases. Will fix it someday later.

@redhoyasa
Copy link
Owner

I just made them synchronized.

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

Successfully merging a pull request may close this issue.

2 participants