Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tree-Shaking for fields #769

Closed
liril-net opened this issue Apr 3, 2024 · 3 comments
Closed

Tree-Shaking for fields #769

liril-net opened this issue Apr 3, 2024 · 3 comments

Comments

@liril-net
Copy link

Hi there, i am working with this fantastic package in my project, but i found it not works with tree-shaking and as project grows, it becomes so large for the output files.

I find that maybe the static readonly fields: FieldList make the rollup tree-shaking not works as it initialized.

Can we make it as a function such as getFields() to make it lazy init to let the tree-shaking works?

@timostamm
Copy link
Member

timostamm commented Apr 3, 2024

Hey Cyril, I think you're right that the call to newFieldList in generated TS can impact tree-shaking:

static readonly fields: FieldList = proto3.util.newFieldList(() => [

I believe that adding a /*@__PURE__*/ annotation right before the call expression should help (see #470 for reference).

But before we make this change, we need to verify that it actually works as expected (previous changes were tested with https://github.com/connectrpc/examples-es/tree/main/bundle-size). This will take us a bit to look into, but it looks worthwhile to me.

Thanks for filing the issue!

@smaye81
Copy link
Member

smaye81 commented Apr 15, 2024

Hi @liril-net. We tested adding this annotation to the newFieldList call, but we aren't seeing any effect with Rollup (or most other bundlers). The only bundler that sees an improvement is esbuild. The PR we're using to test is here.

Are you noticing anything different with Rollup? Or do you see anything missing from how we are bundling in our tests?

@timostamm
Copy link
Member

Let's close this. See connectrpc/examples-es#1499 (comment) for details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants