fix: react tree shaking and non-react entrypoint tree shaking

[KonnorRogers]

😮‍💨 I think I got the treeshaking stuff down. Brief checks with Vite + Webpack builds seem to work.

This does introduce the caveat if you import a Shoelace component and a React component, you're doubling your build size. But I think this is a fair tradeoff.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
shoelace	✅ Ready (Inspect)	Visit Preview	Aug 1, 2023 9:39pm

[daKmoR]

for what it's worth - this also fixes my rollup build (which currently tree shakes almost all components)

PS: if I just remove the sideEffects block all together then rollup is smart enough to handle it correctly
PPS: these type of issues is exactly my past experience when using sideEffects in package.json - why do we define them? I thought most modern tools don't need it anymore? (note: webpack 4 still needs it)

[KonnorRogers]

The short answer is it was the only way to get tree shaking in React main entrypoint...something is triggering side effects, but I can't figure out what.

I believe it has something to do with re-exports.

evanw/esbuild#613
rollup/rollup#305

Given infinite time, I'd love to track down where the sideEffects are coming from, but the tools dont make it simple. The closest I've found is from Rollup.

https://rollupjs.org/configuration-options/#experimentallogsideeffects

[KonnorRogers]

@daKmoR here's an updated thread of things that would need to change so we can get rid of this nasty sideEffects key:

https://twitter.com/RogersKonnor/status/1686774869218770944

Heres the summed up version of things I've identified so far:

• LocalizeController
• Decorators
• static thing = "thing"

LocalizeController is a fairly simple fix. Mark it as pure, carry on.

Decorators. This one's tough. Related: evanw/esbuild#649

The TLDR is we'd need an build.onEnd plugin to iterate over all the built files and check for decorators and wrap them in an IIFE and mark them as pure like this package for Vite:

https://www.npmjs.com/package/vite-plugin-tree-shakable-decorators

static thing = "thing" This one's easy. Just bump ESBuild past 0.18.2, change the target to "esnext", and itll use a static {} block.

EDIT: static {} fields are considered non-treeshakeable by Rollup.

https://twitter.com/RogersKonnor/status/1686793995245023246

[daKmoR]

LocalizeController is a fairly simple fix. Mark it as pure, carry on.

but it's not pure... it has a side effect for the observer
https://github.com/shoelace-style/localize/blob/f03c84581069c549e18e936aa1594094402a9c42/src/index.ts#L28

you could put that in a setup function and then force every app to call it... but that is rather cumbersome 😅
but if you leave it with a side effect then it means every component using localize is NOT side effect free

Decorators

yeah transpilation is alway fun as long as it works... not so fun if it doesn't 😅

Note

Mind my ignorance but may I ask why tree shaking is so important?
if you have dedicated entrypoints for each component and people only import what they need then tree shaking is imho not that important (especially if a lot of component have a localize side effect)

I am just guessing here... but is it because of a single big bundle for react? 🤔 could that be split out?

just tossing ideas - feel free to ignore 🤗

[KonnorRogers]

@daKmoR Nailed it. This was all about the single entrypoint for React. It is technically already split out, we just don't show it in the docs.

In talking with @claviska we've kind of determined that React users should probably have to do the same and cherry pick if they want the smallest possible bundle and to only use what they import.

[claviska]

In talking with @claviska we've kind of determined that React users should probably have to do the same and cherry pick if they want the smallest possible bundle and to only use what they import.

This is unfortunate, but we have to balance the complexity with the cost to solve this and the overall benefit to end users. Non-framework folks cherry pick the same way, so it's really no different — just less idiomatic for React.

[daKmoR]

those kind of decision are always hard 🙈

imho the right call 👍

and if you ever find yourself with too much time at hand 😬 then you can work on a new major version that is fully side effect free...

maybe Chat GTP can do it for us in 2050 🤣

[KonnorRogers]

Closing this. The complexity and increased overhead is not worth it. Will be removing the sideEffects key in a future PR.