Open alehechka opened 2 years ago
Adam, thanks for raising this, we've been pondering it as well. For most effective tree-shaking, it would be ideal to export a symbol for each individual RPC. On the other hand, a service containing a set of methods is the most natural mapping from protobuf sources, so we went with that.
There is also the option to use async imports - types would still be available at build time, but the actual message definitions could be split into a separate bundle that is loaded on demand at run time.
Thanks for the quick response! The reasoning you gave makes sense. I do have one question however.
When testing different setups with the proto definition to figure out the line for tree shaking, I did test having two different services with their own methods defined within one proto file. The generated TS code then had both services defined with the one set of TS files with the respective file name. However, even with two different services defined in the same TS file, and only one getting imported and used in the client-side code, both services and their methods were bundled into the production build.
I put together a simple example repo with slimmed down proto definitions with what I mean here: https://github.com/alehechka/connect-web-tree-shaking
Under /proto/api/example/v1 you can see there are two different proto files, each with their own messages and services. The example.proto
contains two different services each with their own messages for request/response.
The code is then generated under /src/proto/gen/ts/example/v1.
Finally, within /src/App.tsx, I am importing and using exclusively the ExampleService
.
However, after running the production vite build, both the ExampleService
and HelloService
are included in the bundle (available at /dist/assets/index.b2b3071a.js)
For your example.proto, we generate two declarations:
export const ExampleService = { ... };
export const HelloService = { ... };
Let's say you bundle your application from an entry point index.ts
:
import {ExampleService} from "example_connectweb.js";
console.log(ExampleService);
Any sensible implementation of a tree-shaker will eliminate HelloService
from the bundle, and it is indeed not present in the bundle you linked to.
I do see HelloRequest
and HelloResponse
in the bundle, and it is not clear to me why that would be the case. We have the following import statement in example_connectweb.ts
:
import {ExampleRequest, ExampleResponse, HelloRequest, HelloResponse} from "./example_pb.js";
I suspect that the tree shaker is overly cautious here for an unknown reason. It would be really cool to analyze this a bit (splitting up the import statement into 4 individual ones, for a start), and clarify with the tree shaker authors why HelloRequest
and HelloResponse
are not elided, and whether this could be improved in the shaking algorithm. Perhaps this is something you'd like to investigate?
Ah, I see now that HelloService
was removed, just the HelloRequest
and HelloResponse
messages included.
I did some quick tests with the separated imports:
import { ExampleRequest } from './example_pb.js';
import { ExampleResponse } from './example_pb.js';
import { HelloRequest } from './example_pb.js';
import { HelloResponse } from './example_pb.js';
Even with them separated, all four are still included while only using the ExampleService
. I then dug deeper by changing the HelloService
to not use HelloRequest
and HelloResponse
and remove their imports. Unfortunately, the messages were still included in the bundle.
It wasn't until I also removed usage of ExampleRequest
and ExampleResponse
in the ExampleService
that it was finally able to tree shake all of these messages out of the bundle.
This is very curious, because like you said, these messages are all individually exported and should be easily tree shaken if they are not used. If I have some time, I'll try and look into the tree shaking algorithm or clarify with the authors what might be happening here.
I'm also running into this issue. We end up with a 152kb package that just handles our generated connect-web code 😬 I think I may put it in a manual chunk until we figure out something better 🤷
@mckelveygreg, if you are generating to a package, make sure to set "sideEffects": false
.
Depending on your set of protobuf files, it may also pay off to generate .js
+ .d.ts
instead of .ts
- for example, for example.proto, this will only generate the bare minimum of data needed to parse a message.
Also note that even gzip
is pretty good at reducing generated code to a small fraction of the uncompressed bundle.
@mckelveygreg, there is also another option that may or may not be useful, depending on your use case: v1.10.0 of the Buf CLI added a flag that basically adds tree-shaking before code generation. For example, the following command generates only the rpc Say
and the messages required for it:
buf generate buf.build/bufbuild/eliza --include-types buf.connect.demo.eliza.v1.ElizaService.Say
Is your feature request related to a problem? Please describe. When using Vite to produce the production build of a React app, it was found that tree-shaking only occurs per file instead of per function within each file.
In my repository, I have two different proto files, each with its own service. When using the generator for connect-web, it results in respective TS files for each proto file. Currently, I have not imported any code from one of the proto files, and only one method from the other. After building the production build with Vite and inspecting the bundle, there were no methods from the unused proto definition (as expected). However, all methods from the proto definition, in which I'm only using one method, were included in the bundle.
In my case, the service that I am only using one method of in the frontend, I am using the others for inter-service communication in the backend. Those other methods are not accessible over the internet, but it is still a concern that all the details of those methods are sent in the JS bundle.
To get around this, I am breaking off the service methods that are for the frontend into their own proto definition file where the production build will only include these methods in the final bundle.
Describe the solution you'd like Have the generated connect-web TS code be better accessible for fine-grained tree shaking at a per function/method level instead of entire files.
Describe alternatives you've considered As mentioned earlier, I am breaking the methods I want to use in the frontend into their own service and proto definition file where they will be built into their own TS files in the package and thus everything will be tree shaken out.