Add support for @grpc/grpc-js #236

badsyntax · 2020-06-16T07:06:11Z

Add support for generating files that import @grpc/grpc-js instead of grpc. This does not change the default behaviour of the package, but instead introduces a new param allowing you to opt-in to using @grpc/grpc-js.

Changes

Add new mode param, for example:

--ts_out=service=grpc-node,mode=grpc-js

Add generated examples
Update README

Verification

I've only tested it via the generated examples. It's very similar to service=grpc-node other than changing the import paths.

Add mode=grpc-js example

laike9m · 2020-07-03T03:05:26Z

BTW, I tried installing the grpc-node-mode branch using

npm install badsyntax/ts-protoc-gen#grpc-node-mode

ran it, and got this error:

internal/modules/cjs/loader.js:969
  throw err;
  ^

Error: Cannot find module '../lib/index'

It seems the lib directory does not exist in ts-protoc-gen under node_modules.

badsyntax · 2020-07-03T06:56:03Z

The lib directory is only built on prepublishOnly (before the package is published to npm), so installing via git won't work, see:

ts-protoc-gen/package.json

Line 15 in 3e58c65

"prepublishOnly": "npm run build"

laike9m · 2020-07-03T07:03:37Z

Thanks for the quick response. Any suggestions on what I should do to make it work? Or should I stick with sed for now like https://github.com/badsyntax/grpc-js-types?

avdeev · 2020-07-03T07:05:43Z

Quick fix:

sed -i -- 's/import \* as grpc from "grpc";/import \* as grpc from "@grpc\/grpc-js";/g' *

badsyntax · 2020-07-03T07:06:36Z

if you want to continue using this package, then you're going to have to stick with sed. but as mentioned here grpc/grpc-node#1380 (comment), I would suggest to try the new typescript generator offered by the author of grpc-js

andreyctkn · 2020-09-09T15:49:48Z

Hi @badsyntax, Any plans to merge this PR?

badsyntax · 2020-09-09T18:33:31Z

I can't merge as I'm not the owner of this repo. Not sure why I haven't got a review yet :(

andreyctkn · 2020-09-09T21:00:43Z

@MarcusLongmuir Can you please help?

MarcusLongmuir

Thanks for the contribution! Apologies for the long delay in waiting for a review. This had gone under the radar.

Only a few minor things 👍

MarcusLongmuir · 2020-09-09T21:18:26Z

src/service/grpcnode.ts

@@ -4,12 +4,12 @@ import {FileDescriptorProto} from "google-protobuf/google/protobuf/descriptor_pb
 import {CodeGeneratorResponse} from "google-protobuf/google/protobuf/compiler/plugin_pb";
 import {createFile, RPCDescriptor, GrpcServiceDescriptor, RPCMethodDescriptor} from "./common";

-export function generateGrpcNodeService(filename: string, descriptor: FileDescriptorProto, exportMap: ExportMap): CodeGeneratorResponse.File {
+export function generateGrpcNodeService(filename: string, descriptor: FileDescriptorProto, exportMap: ExportMap, modeParameter: string | undefined): CodeGeneratorResponse.File {


This function is only called from the index.ts file so the argument could be an enum rather than an optional string to make the mode explicit.

MarcusLongmuir · 2020-09-09T21:27:53Z

src/index.ts


-    if (parameter === "service=true") {
+    if (service === "true") {


The service parameter should probably have more validation and helpful warnings, but this additional parameter makes that even clearer.

Can you add checks for the supported service and mode values and change this to throw exceptions if they're unrecognised to help users more quickly identify misconfigurations please?

badsyntax · 2020-09-10T19:20:00Z

I've made those changes:

Running:

protoc --plugin="protoc-gen-ts=${PROTOC_GEN_TS_PATH}" --plugin=protoc-gen-grpc=${PROTOC_GEN_GRPC_PATH} --js_out="import_style=commonjs,binary:${OUT_DIR}" --ts_out="service=grpc-nodde:${OUT_DIR}" --grpc_out="${OUT_DIR}" proto/examplecom/simple.proto

Shows:

protoc-gen-ts error: Error: Unrecognised service parameter: grpc-nodde
    at getServiceParameter (/Users/richardwillis/Projects/badsyntax/ts-protoc-gen/lib/util.js:155:19)
    at Object.getParameterEnums (/Users/richardwillis/Projects/badsyntax/ts-protoc-gen/lib/util.js:173:18)
    at /Users/richardwillis/Projects/badsyntax/ts-protoc-gen/lib/index.js:19:25
    at Socket.<anonymous> (/Users/richardwillis/Projects/badsyntax/ts-protoc-gen/lib/util.js:74:9)
    at Socket.emit (events.js:215:7)
    at endReadableNT (_stream_readable.js:1183:12)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)

--ts_out: protoc-gen-ts: Plugin failed with status code 1.

Running:

protoc --plugin="protoc-gen-ts=${PROTOC_GEN_TS_PATH}" --plugin=protoc-gen-grpc=${PROTOC_GEN_GRPC_PATH} --js_out="import_style=commonjs,binary:${OUT_DIR}" --ts_out="service=grpc-node,mode=grpc-jss:${OUT_DIR}" --grpc_out="${OUT_DIR}" proto/examplecom/simple.proto

Shows

protoc-gen-ts error: Error: Unrecognised mode parameter: grpc-jss
    at getModeParameter (/Users/richardwillis/Projects/badsyntax/ts-protoc-gen/lib/util.js:166:19)
    at Object.getParameterEnums (/Users/richardwillis/Projects/badsyntax/ts-protoc-gen/lib/util.js:174:15)
    at /Users/richardwillis/Projects/badsyntax/ts-protoc-gen/lib/index.js:19:25
    at Socket.<anonymous> (/Users/richardwillis/Projects/badsyntax/ts-protoc-gen/lib/util.js:74:9)
    at Socket.emit (events.js:215:7)
    at endReadableNT (_stream_readable.js:1183:12)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)

--ts_out: protoc-gen-ts: Plugin failed with status code 1.

MarcusLongmuir

LGTM. Thanks for making those changes

improbable-prow-robot · 2020-09-10T19:51:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: avdeev, MarcusLongmuir, qingzhenyun
To complete the pull request process, please assign
You can assign the PR to them by writing /assign in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

badsyntax · 2020-09-10T20:02:25Z

Awesome, thanks!

avdeev · 2020-09-10T21:21:03Z

@badsyntax Thanks! Could you release it?

andreyctkn · 2020-09-10T21:22:14Z

thanks !

MarcusLongmuir · 2020-09-10T21:28:14Z

Unfortunately the repo and org setup appears to make that quite difficult without another maintainer so I'll have to do it in the morning when others come online.

If you want to validate usage in the meantime there is now a published version on npm: https://www.npmjs.com/package/ts-protoc-gen/v/0.12.1-pre.6383416.

I'll comment on this PR when the release goes out.

MarcusLongmuir · 2020-09-11T09:22:19Z

0.13.0 is now out: https://www.npmjs.com/package/ts-protoc-gen/v/0.13.0

badsyntax · 2020-09-11T19:19:54Z

Thanks for the release. I've tested the change in my project and it lgtm 👍

badsyntax added 4 commits June 16, 2020 07:56

Add support for grpc_js mode for grpc-node service

e47b372

Add examples for grpc-js mode

69c3f21

Update generated example for grpc-node

07cf3b8

Use querystring package to parse the parameters

db22c75

badsyntax marked this pull request as ready for review June 16, 2020 07:22

Fix linting

b839c0e

badsyntax mentioned this pull request Jun 16, 2020

fix: user @grpc/grp-js instead of grpc #234

Closed

Update README

3e58c65

Add mode=grpc-js example

badsyntax mentioned this pull request Jun 16, 2020

@grpc/grpc-js Support #226

Closed

avdeev approved these changes Jun 29, 2020

View reviewed changes

laike9m mentioned this pull request Jul 3, 2020

grpc-tools: add grpc_js param to grpc_out grpc/grpc-node#1380

Merged

badsyntax mentioned this pull request Jul 3, 2020

Update example for grpc-tools v1.9 badsyntax/grpc-js-typescript#2

Closed

qingzhenyun approved these changes Jul 23, 2020

View reviewed changes

MarcusLongmuir requested changes Sep 9, 2020

View reviewed changes

Validate parameters and use enums instead of strings

30751e5

badsyntax force-pushed the grpc-node-mode branch from ff7a82f to 30751e5 Compare September 10, 2020 19:14

Fix linting

121e17e

badsyntax added 2 commits September 10, 2020 20:30

Update grpc-js usage instructions

2472577

Fix whitespace

e98fab6

MarcusLongmuir approved these changes Sep 10, 2020

View reviewed changes

MarcusLongmuir merged commit 6383416 into improbable-eng:master Sep 10, 2020

badsyntax deleted the grpc-node-mode branch September 10, 2020 19:57

badsyntax mentioned this pull request Sep 11, 2020

Use ts-protoc-gen mode to use grpc-js microsoft/vscode-gradle#690

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for @grpc/grpc-js #236

Add support for @grpc/grpc-js #236

badsyntax commented Jun 16, 2020 •

edited

Loading

laike9m commented Jul 3, 2020 •

edited

Loading

badsyntax commented Jul 3, 2020

laike9m commented Jul 3, 2020

avdeev commented Jul 3, 2020

badsyntax commented Jul 3, 2020

andreyctkn commented Sep 9, 2020

badsyntax commented Sep 9, 2020

andreyctkn commented Sep 9, 2020

MarcusLongmuir left a comment

MarcusLongmuir Sep 9, 2020 •

edited

Loading

MarcusLongmuir Sep 9, 2020

badsyntax commented Sep 10, 2020 •

edited

Loading

MarcusLongmuir left a comment

improbable-prow-robot commented Sep 10, 2020

badsyntax commented Sep 10, 2020

avdeev commented Sep 10, 2020

andreyctkn commented Sep 10, 2020

MarcusLongmuir commented Sep 10, 2020

MarcusLongmuir commented Sep 11, 2020

badsyntax commented Sep 11, 2020


		if (parameter === "service=true") {
		if (service === "true") {

Add support for @grpc/grpc-js #236

Add support for @grpc/grpc-js #236

Conversation

badsyntax commented Jun 16, 2020 • edited Loading

Changes

Verification

laike9m commented Jul 3, 2020 • edited Loading

badsyntax commented Jul 3, 2020

laike9m commented Jul 3, 2020

avdeev commented Jul 3, 2020

badsyntax commented Jul 3, 2020

andreyctkn commented Sep 9, 2020

badsyntax commented Sep 9, 2020

andreyctkn commented Sep 9, 2020

MarcusLongmuir left a comment

Choose a reason for hiding this comment

MarcusLongmuir Sep 9, 2020 • edited Loading

Choose a reason for hiding this comment

MarcusLongmuir Sep 9, 2020

Choose a reason for hiding this comment

badsyntax commented Sep 10, 2020 • edited Loading

MarcusLongmuir left a comment

Choose a reason for hiding this comment

improbable-prow-robot commented Sep 10, 2020

badsyntax commented Sep 10, 2020

avdeev commented Sep 10, 2020

andreyctkn commented Sep 10, 2020

MarcusLongmuir commented Sep 10, 2020

MarcusLongmuir commented Sep 11, 2020

badsyntax commented Sep 11, 2020

badsyntax commented Jun 16, 2020 •

edited

Loading

laike9m commented Jul 3, 2020 •

edited

Loading

MarcusLongmuir Sep 9, 2020 •

edited

Loading

badsyntax commented Sep 10, 2020 •

edited

Loading