search

grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.56k stars 762 forks source link

Use consistent names for generated stubs in typescript. #1168

Closed xinxiao closed 2 years ago

stanley-cheung commented 2 years ago

Sorry for not reviewing this earlier. I thought I might disagree on this one but I could be wrong (or have outdated/incorrect info in the past).

I thought that it's more common for TS project to have filenames more like EchoServiceClientPb.ts rather than the commonJS underscore style.

Also, this might not be as big as a consistency issue in that - most projects using this will pick one style (commonJS vs TS) over another and won't mix. So if they all use --import_style=typescript, then they are consistent within that. I don't see anyone should try to mix --import_style=typescript with --import_style=commonjs+dts within the same project.

Plus, this will be a breaking change. Even though TypeScript support here in this repo is marked as experimental, this doesn't seem like it's worth making a breaking change. So unless TS projects actually use the understore style filenames, I'd rather keep the status quo.

sampajano commented 2 years ago

Thanks for catching the TS style convention, Stanley. :)

@xinxiao sorry that i didn't catch this in the discussion in https://github.com/grpc/grpc-web/issues/1162. If Stanley's comment makes sense to you then maybe we can drop this..

(One alternative discussed is to convert both to EchoServiceClientPb style but then it creates inconsistency with commonjs import style and also a breaking change for users to migrate so it doesn't seem ideal either.)

xinxiao commented 2 years ago

Makes sense; I'll close this PR.