Skip to content

Generate only necessary JS protos#59817

Merged
ravicious merged 2 commits intomasterfrom
r7s/js-proto-deps
Oct 2, 2025
Merged

Generate only necessary JS protos#59817
ravicious merged 2 commits intomasterfrom
r7s/js-proto-deps

Conversation

@ravicious
Copy link
Copy Markdown
Member

@ravicious ravicious commented Oct 1, 2025

protobuf-ts has the generate_dependencies option:

By default, only the PROTO_FILES passed as input to protoc are generated, not the files they import (with the exception of well-known types which are always generated when imported). Set this option to generate code for dependencies too.

In the past, we imported some protos from api/proto/teleport in proto/teleport/lib/teleterm. We had to add a couple of additional inputs in buf-ts.gen.yaml, otherwise Vite would fail when trying to compile the generated protos – they'd try to import paths that don't exist.

With this option, we can specify as inputs only the paths that we really need and protobuf-ts will automatically pull in any imported protos. As you can see in the diff, it reduces the amount of generated code.

I verified that this doesn't break anything by running pnpm build-term, pnpm build-ui-oss, and pnpm build-ui-e. If any actually used files were missing, those commands would fail.

In the next PR, I'm going to move web/packages/teleterm/src/sharedProcess/api/proto/ptyHostService.proto to proto/teleport/web/teleterm.

@ravicious ravicious requested a review from espadolini October 1, 2025 11:13
@github-actions github-actions Bot requested review from bl-nero and eriktate October 1, 2025 11:14
@ravicious ravicious added the no-changelog Indicates that a PR does not require a changelog entry label Oct 1, 2025
@ravicious ravicious removed the request for review from eriktate October 1, 2025 11:15
Comment thread buf-ts.gen.yaml
Comment on lines +33 to +35
# By default, only the proto files passed as input to protoc are generated, not the files they
# import (with the exception of well-known types which are always generated when imported).
# generate_dependencies generates code for dependencies too.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that the wording here is wrong; we are still passing the whole directory of compiled protos to the plugin like before, this option lets the plugin figure out what to generate based on the list of files we are asking it to generate (in the paths list) instead of having to trim down the list ourselves.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is copied from protobuf-ts docs, minus the last paragraph. 😏 We no longer have to manually include dependencies in the list so I think the comment is correct.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see where their interpretation and my misunderstanding comes from; by "the proto files passed as input" they mean the list of proto files - all dependencies are always compiled and passed to the plugin. I doublechecked the implementation because the wording makes it seem like protobuf-ts goes and reads extra files independently, which thankfully is not what's happening.

@ravicious ravicious enabled auto-merge October 2, 2025 09:17
@ravicious ravicious added this pull request to the merge queue Oct 2, 2025
Merged via the queue into master with commit 013d8e4 Oct 2, 2025
44 of 48 checks passed
@ravicious ravicious deleted the r7s/js-proto-deps branch October 2, 2025 09:42
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@ravicious See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Failed
branch/v18 Create PR

rhammonds-teleport pushed a commit that referenced this pull request Nov 6, 2025
* Automatically generate code for imported protos

* Put protobuf-ts opts in alphabetical order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v17 backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants