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

Experimental Typescript support #258

Merged
merged 1 commit into from
Aug 24, 2018

Conversation

stanley-cheung
Copy link
Collaborator

For #255

I am going to try to expand on this. For my very initial understanding, this seems to work.

Feedback highly appreciated!

cc: @jonahbron

@stanley-cheung
Copy link
Collaborator Author

stanley-cheung commented Aug 22, 2018

So far, this PR demos how a typescript project can make use of the index.d.ts typings for the grpc-web module. I hand-wrote an echo_grpc_web_pb.ts file that could be generated from a future grpc-web x TS protoc plugin. To test whether the index.d.ts typings do matter, I flip, let's say, a method parameter from a string to a number. Now the tsc command will fail because a string is being passed.

To run:

$ docker-compose build ts-client
$ docker run -it --rm grpc-web:ts-client /bin/bash
$ cd /github/grpc-web/net/grpc/gateway/examples/echo/ts-example
$ node dist/client.js

@jonahbron
Copy link
Collaborator

That looks right to me. One alternative to the echo_grpc_web_pb.ts file I've seen in ts-protoc-gen is producing .d.ts for the generated files too. That way the JS generator can still be used as-is.

}

export interface GrpcWebClientBaseOptions {
format?: string;
Copy link

@aberasarte aberasarte Aug 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we could add the suppressCorsPreflight option to the GrpcWebClientBaseOptions interface as well:

suppressCorsPreflight?: boolean

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do.

@stanley-cheung
Copy link
Collaborator Author

That looks right to me. One alternative to the echo_grpc_web_pb.ts file I've seen in ts-protoc-gen is producing .d.ts for the generated files too. That way the JS generator can still be used as-is.

Will try that. Thanks.

@stanley-cheung
Copy link
Collaborator Author

stanley-cheung commented Aug 23, 2018

Added a full Typescript example up to this point.

ts-example/EchoServiceClient.ts: a hand-written would-be full-TS codegen output

ts-example/echo_grpc_web_pb.d.ts: .d.ts typings that are meant to work alongside existing echo_grpc_web_pb.js CommonJS codegen output
ts-example/echo_pb.d.ts: .d.ts typings for protobuf message types, meant to work alongside echo_pb.js like above

ts-example/client.ts: TS client app code

Uncomment either Line 25 or Line 26 to try between the two modes above:

import {EchoServiceClient} from './echo_grpc_web_pb';   // .d.ts typings for existing JS codegen output
// import {EchoServiceClient} from './EchoServiceClient';  // full-TS codegen output

@stanley-cheung stanley-cheung changed the title Add index.d.ts typings file Experimental Typescript support Aug 24, 2018
@stanley-cheung
Copy link
Collaborator Author

Added 2 modes to the protoc-gen-grpc-web protoc plugin

  • import_style=commonjs+dts: this will generate .d.ts typings for both the <proto>_grpc_web_pb.js service stub and the <proto>_pb.js proto message classes
  • import_style=typescript: this will generate <proto>ServiceClientPb.ts typescript service stub

@stanley-cheung
Copy link
Collaborator Author

Squashing commits. Got internal reviewed. Will merge this soon.

@stanley-cheung stanley-cheung merged commit a180725 into grpc:master Aug 24, 2018
@stanley-cheung stanley-cheung deleted the typescript-support branch August 24, 2018 23:10
loyalpartner pushed a commit to loyalpartner/grpc-web that referenced this pull request Sep 4, 2020
- ts-protoc-gen clears out the callbacks anyways
- Fixes grpc#257
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants