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-tools: add grpc_js param to grpc_out #1380

Merged
merged 1 commit into from
May 11, 2020

Conversation

badsyntax
Copy link
Contributor

@badsyntax badsyntax commented Apr 22, 2020

This is a minor change to use the pure JavaScript gRPC Client @grpc/grpc-js instead of the (now deprecated) grpc node package when generating javascript files using the proto compiler. This change is very similar to the generate_package_definition param.

Reasoning:

I understand that it's recommended to use @grpc/proto-loader to the load protos files.

I have some hesitation to adopt this approach, as when using TypeScript, I have to typecast using type assertions to type the grpc client as a ServiceClientConstructor, for example:

const echoService = loadProtoFile(protoFile)
      .EchoService as ServiceClientConstructor;

My client is then defined as:

export interface ServiceClient extends Client {
[methodName: string]: Function;
}

..and I loose all the benefits of (for example) IntelliSense in my IDE. I also don't know if my method calls are correct until I compile the TypeScript to JS. It's not a great developer experience.

The change i'm proposing is to support "grpc_js" within the proto compiler. This will allow me to generate static javascript files that require @grpc/grpc-js instead of grpc, and will allow existing tools to then generate types from those files.

I also feel this a good change to support existing projects & tooling that would like to switch over to using @grpc/grpc-js. It will allow for better adoption as the required changes in the consuming projects are minimal.

This is a minor change to use the pure JavaScript gRPC Client `@grpc/grpc-js`
instead of the (now deprecated `grpc` node package).
@laike9m
Copy link

laike9m commented Jul 3, 2020

Hi Richard, you mentioned

This will allow me to generate static javascript files that require @grpc/grpc-js instead of grpc, and will allow existing tools to then generate types from those files.

May I ask what tools you're using to generate TS types? I tried grpc_tools_node_protoc_ts but it does not seem to support grpc-js. Thanks.

@laike9m
Copy link

laike9m commented Jul 3, 2020

Never mind. I saw your PR: improbable-eng/ts-protoc-gen#236

Hope it gets merged soon.

@badsyntax
Copy link
Contributor Author

@laike9m i'm not sure if the authors of that package are keen to improve it as i've had zero feedback.

i've been meaning to have a look at the new typescript generator offered by @murgatroid99, and i would suggest to consider using that package instead.

see: #528 (comment)

example of how it can be used:

./node_modules/.bin/proto-loader-gen-types --keepCase --longs String --enums String --defaults --oneofs --json\
--includeDirs deps/envoy-api/ deps/udpa/ node_modules/protobufjs/ deps/googleapis/ deps/protoc-gen-validate/ \
-O src/generated/ --grpcLib ../index envoy/service/discovery/v2/ads.proto envoy/api/v2/listener.proto envoy/api/v2/route.proto envoy/api/v2/cluster.proto envoy/api/v2/endpoint.proto

@laike9m
Copy link

laike9m commented Jul 3, 2020

Great, let me try that out. Thanks for the pointer.

@laike9m
Copy link

laike9m commented Jul 3, 2020

@laike9m i'm not sure if the authors of that package are keen to improve it as i've had zero feedback.

i've been meaning to have a look at the new typescript generator offered by @murgatroid99, and i would suggest to consider using that package instead.

see: #528 (comment)

example of how it can be used:

./node_modules/.bin/proto-loader-gen-types --keepCase --longs String --enums String --defaults --oneofs --json\
--includeDirs deps/envoy-api/ deps/udpa/ node_modules/protobufjs/ deps/googleapis/ deps/protoc-gen-validate/ \
-O src/generated/ --grpcLib ../index envoy/service/discovery/v2/ads.proto envoy/api/v2/listener.proto envoy/api/v2/route.proto envoy/api/v2/cluster.proto envoy/api/v2/endpoint.proto

I gave it a try, but somehow it confused me. Not sure if I'm doing anything wrong:
#528 (comment)

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 this pull request may close these issues.

4 participants