protocolbuffers / protobuf-javascript

BSD 3-Clause "New" or "Revised" License
362 stars 67 forks source link

Generated code with extensions does not build #151

Closed schmidt-sebastian closed 1 year ago

schmidt-sebastian commented 1 year ago

I am using "google-protobuf": "^3.21.2", via Bazel for https://github.com/google/mediapipe

We use extensions in one of our Protobuf files: https://github.com/google/mediapipe/blob/master/mediapipe/framework/calculator_options.proto

These are used like this: https://github.com/google/mediapipe/blob/master/mediapipe/tasks/cc/vision/gesture_recognizer/proto/gesture_classifier_graph_options.proto#L28

In the generated code, it seems like the class is called calculator_options_pb:

var calculator_options_pb = {};

However, when the extensions are registered, the fully qualified path is used:

mediapipe_framework_calculator_options_pb.CalculatorOptions.extensionsBinary[336783863] = {...}

I can fix all build errors by manually assigning mediapipe_framework_calculator_options_pb to calculator_options_pb, but I wonder if there is a patch/or setting that can be automatically applied.

dibenede commented 1 year ago

Can you please describe how you're invoking protoc-gen-js? We're a little thrown off to see protobufjs and protoc-gen-ts in your package.json. Also, we're not sure if rules_closure is compatible with our newer packaging as a protoc plugin.

We are also not familiar with js_proto_library.

If you can share the full generated code, that could also be of use.

schmidt-sebastian commented 1 year ago

It's coming from here: https://github.com/rules-proto-grpc/rules_proto_grpc/blob/master/js/js_proto_library.bzl#L10

The actual invocation is triggered by protoc-gen-ts, which looks like this: https://gist.github.com/schmidt-sebastian/bdcb716a9c165c41743e8fedc706c501

I tried to get the actual underlying command, but so far I have been unsuccessful (I probably have to fork and patch protoc-gen-ts). I can try some more tomorrow.

schmidt-sebastian commented 1 year ago

This was actually caused by a missing import: https://github.com/google/mediapipe/commit/d84eec387bb277b4f379f360d97bbf734cb3ae13