[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

grpc-tools: add grpc_js param to grpc_out #1380

Merged
merged 1 commit into from
May 11, 2020
Merged

grpc-tools: add grpc_js param to grpc_out #1380

merged 1 commit into from
May 11, 2020

Conversation

badsyntax
Copy link
Contributor
@badsyntax badsyntax commented Apr 22, 2020 •
edited
Loading

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:

grpc-node/packages/grpc-js/src/make-client.ts

Lines 83 to 85 in 4ec023c

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.

@badsyntax
grpc-tools: add grpc_js grpc_out param
f7deff3 
This is a minor change to use the pure JavaScript gRPC Client `@grpc/grpc-js`
instead of the (now deprecated `grpc` node package).
@badsyntax badsyntax mentioned this pull request May 10, 2020
Closed
@murgatroid99 murgatroid99 merged commit fbc5174 into grpc:master May 11, 2020
@murgatroid99 murgatroid99 mentioned this pull request May 12, 2020
Merged
@badsyntax badsyntax mentioned this pull request Jun 15, 2020
Closed
@badsyntax badsyntax deleted the grpc-tools--grpc-js branch June 15, 2020 11:51
@badsyntax badsyntax mentioned this pull request Jun 16, 2020
Merged
@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:

grpc-node/packages/grpc-js/generateTypes.sh

Lines 18 to 20 in 095a3b4

./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 •
edited
Loading

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

@laike9m laike9m mentioned this pull request Jul 3, 2020
Closed
@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:

grpc-node/packages/grpc-js/generateTypes.sh

Lines 18 to 20 in 095a3b4

./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
Reviewers

@murgatroid99 murgatroid99 murgatroid99 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@badsyntax @laike9m @murgatroid99 @kokoro-team