Inconsistent naming method with protobuf entities' "*_pb.d.ts"

[xinxiao]

Should we consider changing the naming convention here to make things more consistent?

grpc-web/javascript/net/grpc/web/generator/grpc_generator.cc

Line 1482 in 5831a96

"ServiceClientPb.ts";

[sampajano]

I kinda agree.. :)

Do you mind send a PR? Thanks :)

[sampajano]

Once that's done, we can probably also clean up the comments here:

grpc-web/net/grpc/gateway/examples/echo/ts-example/client.ts

Lines 22 to 27 in 1efe741

	// Uncomment either one of the following:
	// Option 1: import_style=commonjs+dts
	import {EchoServiceClient} from './echo_grpc_web_pb';
	// Option 2: import_style=typescript
	// import {EchoServiceClient} from './EchoServiceClientPb';

And ideally have our integration tests test both commonjs+dts and typescript import styles.

[xinxiao]

#1168 PR created.

I found it a bit hard to test the change. I tried to run the series of example docker containers to see if the change is valid but,

a. Couldn't find a docker test scenario for --grpc-web_out=import_style=typescript so I modified the case locally for commonjs+dts to fit.
b. The prereqs image only works for x86 machines but sadly what I have is an arm mac :( I wonder if we could use something like bazelisk to wrap a good bazel installment for the testing environment?

[sampajano]

#1168 PR created.

Thanks so much for the PR! 😃

I found it a bit hard to test the change. I tried to run the series of example docker containers to see if the change is valid but,

a. Couldn't find a docker test scenario for --grpc-web_out=import_style=typescript so I modified the case locally for commonjs+dts to fit.

Thanks for doing the local tests! Indeed we don't have integration tests for import_style=typescript, which we probably should... :)

For now, you could probably modify the ts-client Dockerfile and manually run the echo server using docker-compose up --build node-server envoy ts-client, and see if the example still works.

We do have some unit tests like the eval_test and tsc_test that you'd need to update.

b. The prereqs image only works for x86 machines but sadly what I have is an arm mac :( I wonder if we could use something like bazelisk to wrap a good bazel installment for the testing environment?

Interesting.. Are you not able to build the prereqs imagine locally?

What's the error if you run the run_basic_test.sh script (which runs docker-compose build prereqs)?

Since we use Docker i'd thought we wouldn't need something like bazelisk..