ts-proto icon indicating copy to clipboard operation
ts-proto copied to clipboard

Feature Request: grpc/grpc-web client support (in addition to @improbable-eng/grpc-web)

Open kkimdev opened this issue 3 years ago • 5 comments

It seems grpc/grpc-web is more actively maintained nad has more Github Stars than @improbable-eng/grpc-web!

  • https://github.com/grpc/grpc-web/graphs/contributors
  • https://github.com/improbable-eng/grpc-web/graphs/contributors

kkimdev avatar Sep 02 '22 07:09 kkimdev

Hey @kkimdev ; that sounds good to me; if you'd like to submit a PR that'd be great.

Not sure the best flag; like grpcWebImpl=improbable-eng | grpc-io.

Or maybe we should just move completely over to the grpc.io library; that would be a large breaking change, so ideally we could keep both options around.

stephenh avatar Sep 02 '22 14:09 stephenh

Would be cool if it would support grpc/grpc-web. Especially considering that grpc-web is now in maintenance mode https://github.com/improbable-eng/grpc-web/commit/1d9bbb09a0990bdaff0e37499570dbc7d6e58ce8

jaqxues avatar May 03 '23 20:05 jaqxues

What would it take to implement grpc/grpc-web support @stephenh? I need this and would be happy to give it a go, but so far I don't understand how this would be implemented properly. The current grpc-web generation logic seems to be mostly in main.ts and generate-services.ts, is that right? How would what need to change to fit into the current system?

flotothemoon avatar Dec 30 '23 16:12 flotothemoon

Hi @flotothemoon ! I believe that main.ts checks for outputClientImpl=grpc-web and then calls functions that are defined in generate-grpc-web.ts for the actual grpc-web-specific output.

In terms of approach, I would probably:

  • Pick an existing integration test like integration/grpc-web, and copy/paste it over into integration/grpc-js-web (or some better name?)
  • Add a outputClientImpl=grpc-jc-web (or again a better name)
  • Copy/paste generate-grpc-web.ts to generate-grpc-js-web.ts
  • Take the current improbable-eng based output and just manually convert it over to work with grpc-web instead; keep this handy as your goal state / desired output
  • Gradually refactor genericate-grpc-js-web.ts over to output your desired grpc-web-based output

As a disclaimer maybe it'd be simpler to start generate-grpc-js-web.ts as a fresh file, like if it's dramatically different from the current improbably-eng-based output, but I'm not really sure what/how much needs to change.

But that's how I'd approach it!

stephenh avatar Dec 30 '23 16:12 stephenh

I just found https://github.com/timostamm/protobuf-ts for generating TS types and it already integrates with grpc/grpc-web, so I'll give that a go instead for now - but thanks for the pointers, I may get back to this!

flotothemoon avatar Jan 05 '24 18:01 flotothemoon