Add /*@__PURE__*/ annotation for better tree-shaking support

[pmzi]

Hi,

We are using protoc-gen-es to generate TypeScript files for our protobuf messages. In our project, there are unused messages that get compiled into JavaScript files but are not imported anywhere. Since we're using Vite, which relies on Rollup for bundling, Rollup does not remove makeMessageType function calls from the output bundle, as seen in the example below:

f.makeMessageType("blacklist.SearchUnblacklistsByPhonesResponse", () => [{
  no: 1,
  name: "unblacklisted_phones",
  kind: "scalar",
  T: 9,
  repeated: !0
}]);

Rollup assumes that f.makeMessageType may have side effects (such as mutating the global state), leading to unnecessary code being included in the final bundle. As a result, our bundle size has doubled, becoming significantly larger.

To address this issue, I've added the /*@__PURE__*/ annotation before makeMessageType function calls. This informs Rollup that these function calls do not have any side effects, as documented in the Rollup configuration options. Adding this annotation will allow Rollup to better optimize the output bundle by eliminating unused code.

If you have any suggestions for a better solution or improvements, please leave a comment.

Thank you!

[CLAassistant]

All committers have signed the CLA.

[timostamm]

Hey @pmzi, this makes sense in general (and should be applicable to makeEnum too). I think pure annotations were started by mishoo/UglifyJS#1448 and are understood by esbuild as well.

But to add the annotation, we need a good understanding of where it's necessary, and ideally some test to proof we're doing the right thing, or at least a reproducible example.

[pmzi]

Thanks @timostamm for your response.

I will work on a reproducible example for unused message types. Is that what you are asking for?

[smaye81]

Hi @pmzi. Yes, something along those lines. We basically want to understand all the places it's necessary. So coming with a reproducible example as well as some unit tests that verify we're fixing things correctly would definitely help.

[pmzi]

Hi @smaye81,

Okay. I will work on it.

[dae]

Adding @__PURE__ annotations also allows esbuild to omit unused proto messages. Here's a very simple example: https://github.com/ankitects/proto-tree-shaking-example

The same annotation works for makeEnum calls as well.

[pmzi]

I added the annotation for makeEnum calls as well.

Thanks @dae for the example.

[pmzi]

CC: @smaye81 @timostamm

[smaye81]

Hi @pmzi thanks for the update. We'd still like to see some tests to confirm the fix and to make sure nothing regresses in the future.

Granted, the tests probably aren't easy and are going to end up being somewhat ugly, but we don't want to just drop these annotations into the generated code without some sort of verification they're providing a meaningful improvement.

[jcready]

Is the ask that we confirm that the pure comments actually work for tree-shaking purposes (easy-ish, but simply testing some parser/bundler implementation) or that we add tests to ensure that for all code for which we've added a pure comment are, in fact, pure (very difficult: parse generated code, instrument AST to track every mutation/setter, output AST to JS, execute all code for which we've added a pure comment and ensure no unexpected mutation/setter are run)?

[smaye81]

It would be the former, yeah. Just verification that adding the PURE directive allows for correct tree-shaking across bundler implementations.

[smaye81]

@pmzi we did some testing on PURE annotations as part of connectrpc/examples-es#1444. We've verified this works for most popular bundlers and also did some other testing to verify on TS generated code. If you'd like to rebase this and resolve the conflicts, we can merge it in. Thanks again.

[pmzi]

@smaye81, thanks for the update, I just resolved the conflicts.

[pmzi]

Should I do something to fix the CI failure or it's okay? @smaye81

[smaye81]

Yeah, you will need to run make to regenerate the proto files in the repository. Now that the PURE annotation has been added, some generated code has changed, so the checkdiff step is failing. You should simply be able to run make from the repo root.

See our https://github.com/bufbuild/protobuf-es/blob/main/.github/CONTRIBUTING.md docs too for more info.

[pmzi]

@smaye81 done. Thanks for the help.

[timostamm]

This was just released in v1.8.0.