Skip to content
This repository was archived by the owner on Jan 18, 2026. It is now read-only.

fix: change style of comment in protos for protobufjs#150

Closed
alexander-fenster wants to merge 2 commits intomasterfrom
convert-protos
Closed

fix: change style of comment in protos for protobufjs#150
alexander-fenster wants to merge 2 commits intomasterfrom
convert-protos

Conversation

@alexander-fenster
Copy link
Contributor

So this one is weird.


TL;DR: in this PR we convert all // comments to /** ... */ in all the proto files that are copied to the generated client library.


In the monolith generator, we created the whole src/v*/doc folder (e.g. here) that was pretty much the copy of protos with comments, converted to JavaScript for jsdoc.

In the micro-generator, we don't do that. Instead, we run pbjs to generate protos/protos.js and then let jsdoc read that file, which just works.

But, I just realized that jsdoc does not consider // style comments from proto files and does not copy them to the resulting protos/protos.js. Its documentation says here:

ProTip! Documenting your .proto files with /** ... */-blocks or (trailing) /// ... lines translates to generated static code.

So... we must convert the proto comments from // to /** ... */ to make them go into the jsdoc. Luckily, we have a great place to do that - right in the code where we copy proto files into the resulting directory. This PR does exactly that.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 20, 2019
@JustinBeckwith
Copy link
Contributor

Yeah, this is weird 😆 Should we be proposing a change in jsdoc or protobufjs instead? I'm a little worried about trying to accurately parse JavaScript in a way that's complete/effective here.

@alexander-fenster
Copy link
Contributor Author

I'm only parsing protos, not JavaScript, so it's probably easier and more predictable.

jsdoc is out of question, it just reads what is in protos/protos.js. But protobufjs did not put the comments there because they ignore //-style comments.

Fixing this in pbjs may be an option, but a harder one (let me say this - I spent like half an hour staring at that code and did not immediately figure out how to make it work there). So I still opt for local proto rewrite.

Note that we don't really use these protos for anything in the runtime (they are now published just for reference, we don't load them), and if any parsing problem happens, we'll catch it when synthtool fails (when we run npx compileProtos src).

@alexander-fenster
Copy link
Contributor Author

I need to think more about the proper way of doing this, given that we now can make protobuf.js releases.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants