New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generate grpc/pb with commonjs_strict #5336
base: main
Are you sure you want to change the base?
Conversation
I just pulled and built this (w/ |
Thanks - that's part of the root bug, that even the built-in protoc-js-gen doesn't correctly support The good news is that we don't actually use that package or its message types at all presently, so we can't run into this bug with them. If that changes, we're going to need to entirely give up on the existing codegen. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked locally and the only thing in global now is proto.google.protobuf.Timestamp
, but as @niloc132 noted we don't use this.
Ran web e2e tests against this branch and they passed.
Didn't check that the TS gen worked properly, so if that works then this looks good to me
This enables applications to load more than one version of the JS API as ES6 modules, without the implementations interfering with each other. Unfortunately, the JS protoc plugin doesn't support this case well, so workarounds are required to correctly reference proto files that use packages.
Fixes #5318