search

grpc / grpc-web

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

Remove unused imports #1235

Closed meling closed 1 year ago

meling commented 2 years ago

This checks which fields are actually used before adding the import alias. That is, some imports are only used by other languages, e.g., option imports such as protopatch for Go. Such imports are not used by Javascript or Typescript, and thus should not be imported. Currently, they result in errors like:

proto/ag/ag_pb.d.ts:4:30 - error TS2307: Cannot find module '../patch/go_pb' or its corresponding type declarations.

4 import * as patch_go_pb from '../patch/go_pb';

Fixes #529.

sampajano commented 2 years ago

Thanks so much for the fix! 😃

I wonder if you could provide a minimal example in the PR description on when this issue appears and how this fix addresses it?

Also, i wonder if you could consider adding a test for this change? I believe the following place might be a good fit: https://github.com/grpc/grpc-web/blob/master/packages/grpc-web/test/plugin_test.js

Thanks!! :)

meling commented 2 years ago

I'll try to fix these issues. Hopefully later this week.

meling commented 1 year ago

Seems unlikely that I'll get to this anytime soon; my brief initial attempts at preparing a test case were not successful (cannot recall the problem, but I think it was related to setup / tooling). While this worked on my particular use case, it is a bit of a hack, so perhaps a more robust solution could be implemented. Closing, but feel free to reopen if anyone else wants to pick this up.

sampajano commented 1 year ago

Thanks for the effort and explanation @meling! :)