Treeshaking doesn't always work
cutiful opened this issue · 27 comments
@chakra-ui/react shows the same size whether you export a few components or all of them. If you esbuild it locally however, the generated bundle is much smaller.
bundlejs: Bundle size is 920.21KB -> 297.6KB (gzip)
vs
esbuild: out.js is 397229 bytes
The package in question has both CommonJS and ESM exports. main field in package.json points at the CommonJS one. Could that be the issue?
Thanks for reaching out, I'll investigate
@cutiful It seems you're right, the issues seems to CJS. I'll take a closer look at this and get back to you
It seems tree-shaking no longer works in general on the latest version of the site 🤔 . This example used to show a much smaller value before with only that one export, but now includes all exports in the bundle size.
Oh, then this issue might be caused by the latest version of esbuild, I'll downgrade and verify rhe results
I think I now have a solution, I'll test some stuff out and get back to you
I've gotten a better treeshaken version (Uncompressed) from 920.21KB to 734 kB and (Compressed) 297.6KB to 230 kB still WIP, but I theorize that this issue might be due to multiple versions of the same package.
I've found that bundlejs sees @coremirror/state@^6.2.0 and @coremirror/state@^6.1.2 (both versions should use the latest minor release per npm spec e.g. @coremirror/state@^6.1.2 -> @coremirror/state@^6.2.0) but bundlejs currently doesn't pay attention to the redirects causing it to think ^6.1.2 and ^6.2.0 are distinctly different packages. Currently working on a possible fix
After fixing the duplicate version bug, the latest (Uncompressed) size is 610 kB, the (Compressed) size is 201 kB
I can't seem to get it any smaller than that, you can check it out here https://deno.bundlejs.com/?q=@chakra-ui/react&treeshake=[{+ChakraProvider,Container,Input+}].
If you look through the resulting bundle I'm not sure there is more that we can safely treeshake https://deno.bundlejs.com/?q=@chakra-ui/react&treeshake=[{+ChakraProvider,Container,Input+}]&config={"esbuild":{"minify":false}}&file you'll realize it's fairly
Yeah, that's due to a bug I was trying to fix yesterday but ran out of time. Bundlejs currently doesn't continuously check if a directory has a package.json, leading to react-remove-scroll-bar/constants not working properly
This should now hopefully be fixed, please give it a try
It builds now, but treeshaking still doesn't fully work. I uploaded the metafiles from both bundlejs and regular esbuild here. For example, bundlejs seems to include @chakra-ui/slider in full, whereas esbuild tree-shakes it.
They both import it through the same path. Here's what esbuild says about one of the files:
node_modules/@chakra-ui/slider/dist/chunk-URECC76Z.mjs
Original size: 13.5 kb
Bundled size: 0 bytes
Module format: ESM
This file is included in the bundle because:
Output file out.js
Entry point index.js contains:
import "@chakra-ui/react";
Imported file node_modules/@chakra-ui/react/dist/index.mjs contains:
import "@chakra-ui/slider";
Imported file node_modules/@chakra-ui/slider/dist/index.mjs contains:
import "./chunk-URECC76Z.mjs";
So imported file node_modules/@chakra-ui/slider/dist/chunk-URECC76Z.mjs is included in the bundle.
"Included in the bundle", but bundled size is 0 bytes. And here's bundlejs:
http-url:https://unpkg.com/@chakra-ui/slider@2.0.21/dist/chunk-URECC76Z.mjs
Original size: 13.5 kb
Bundled size: 5.4 kb
Module format: ESM
This file is included in the bundle because:
Output file index.js
Entry point virtual-filesystem:/index.tsx contains:
import "@chakra-ui/react";
Imported file http-url:https://unpkg.com/@chakra-ui/react@2.5.1/dist/index.mjs contains:
import "@chakra-ui/slider";
Imported file http-url:https://unpkg.com/@chakra-ui/slider@2.0.21/dist/index.mjs contains:
import "./chunk-URECC76Z.mjs";
So imported file http-url:https://unpkg.com/@chakra-ui/slider@2.0.21/dist/chunk-URECC76Z.mjs is included in the bundle.
I thought it could be some TypeScript/JSX-related bug, but it's the same if I rename the esbuild input file to index.tsx.
Also, there seems to be an issue with bundlejs' built-in analyzer. It shows a much larger bundle size. So I'm using metafiles to compare.
fwiw, tree-shaking seems to be fixed for floating-ui at least
Okay, I don't know what that option actually does. I thought it would include all imports from every file it sees when treeshaking is disabled.
If I do export * from "@chakra-ui/react";, the size is roughly the same in both esbuild and in-browser bundlejs. 625.5kB and 626.33kB (respectively) with treeshaking and 661.7kB and 678.57kB without.
@cutiful The ballooning file size is usually due to bundlejs using the wrong variant for bundling e.g. the node variant instead of the browser variant, etc... The problem is I'm having trouble determing which package it's messing up in.
The bundle analysis is rather large but I'm not sure which one it's messing up in https://deno.bundlejs.com/?q=@chakra-ui/react&treeshake=[{ChakraProvider,Container,Input}]&analyze
@cutiful The cause of the large bundles seem to be wrong version of framer-motion and other packages, e.g. bundlejs is using framer-motion@10, but esbuild is using framer-motion@9, at least that's what was set in the package.json that seems to be part of what's causing the problems
I've been experimenting with potential improvement to treeshaking, please give it a try https://bundle-git-treeshaking-fix-okikio-dev.vercel.app/?q=%40chakra-ui%2Freact&treeshake=%5B%7B+ChakraProvider%2CContainer%2CInput+%7D%5D
I tried building Chakra, Ant and Chart.js with tree-shaking, bundle sizes seem accurate. Thank you so much for your work!
I also noticed that bundlejs makes a lot of repeated network requests. E. g. building Chakra resulted in 411 requests to https://unpkg.com/react@18.2.0/index.js (HEAD and GET). This isn't a critical issue for me, but I thought it's worth reporting.
@cutiful Awesome, though it's still a WIP, ideally esbuild-wasm treeshaking would work just work properly without needing to run twice, I've already created an issue on evanw/esbuild#3129.
The 2 network requests are nesecary to determine which extenstion to use when fetching the file, because depending on the package the import could be ./index.js, ./index.mjs, .mjs, .js, .ts, .mts, etc... in order to determine the right extension I send HEAD requests which are small and faster than GET requests, if the request passes then that's the right package if it doesn't then it is not
That makes sense. It just seems there's a cache for JS files, so I thought there would only be 2 requests to each of them.
Good news, I seem to have a permanent solution to the problem of poor treeshaking. That isn't abysmally slow.
It seems this issue is caused by esbuild being unable to determine if it should include sideEffects or not.
On a local file system esbuild can easily just look up the package.json files but it can't quite do that when using a virtual fs, so the solution is to just let esbuild know to use sideEffects if it's stated in the package.json file.
I'll add this solution to bundlejs as soon as possible
Treeshaking should now work properly across the website and API. I'll keep this issue open, please let me know if you notice any issues
Since the fix seems to work ill close this issue
@okikio sorry, I didn't have time to test. It does seem to be working on the main website now. Thanks again!
No worries, awesome to see that it's working again