okikio / bundlejs

An online tool to quickly bundle & minify your projects, while viewing the compressed gzip/brotli bundle size, all running locally on your browser.
https://bundlejs.com
MIT License
751 stars 13 forks source link

Treeshaking doesn't always work #43

Closed cutiful closed 1 year ago

cutiful commented 1 year ago

@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?

okikio commented 1 year ago

Thanks for reaching out, I'll investigate

okikio commented 1 year ago

@cutiful It seems you're right, the issues seems to CJS. I'll take a closer look at this and get back to you

atomiks commented 1 year ago

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.

okikio commented 1 year ago

Oh, then this issue might be caused by the latest version of esbuild, I'll downgrade and verify rhe results

okikio commented 1 year ago

I think I now have a solution, I'll test some stuff out and get back to you

okikio commented 1 year ago

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

okikio commented 1 year ago

After fixing the duplicate version bug, the latest (Uncompressed) size is 610 kB, the (Compressed) size is 201 kB

okikio commented 1 year ago

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

cutiful commented 1 year ago

Thanks! It doesn’t seem to work for me right now, both UNPKG and jsDelivr return 404 for focus-lock/constants and react-remove-scroll-bar/constants. I tried building Ant Design (https://bundlejs.com/?q=antd&treeshake=%5B%7B+Button+%7D%5D) in the browser version though, it shows >2MB, which isn’t right. Is it running the latest code like the Deno API?

okikio commented 1 year ago

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

okikio commented 1 year ago

This should now hopefully be fixed, please give it a try

cutiful commented 1 year ago

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.

cutiful commented 1 year ago

Interestingly, @chakra-ui/slider isn't included in the esbuild bundle even if I pass --tree-shaking=false to it. It results in a bundle size of 442.9kB. For bundlejs with treeshaking disabled, it's 687kB. This is really confusing.

atomiks commented 1 year ago

fwiw, tree-shaking seems to be fixed for floating-ui at least

cutiful commented 1 year ago

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.

okikio commented 1 year ago

@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

okikio commented 1 year ago

@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

okikio commented 1 year ago

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

cutiful commented 1 year ago

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.

okikio commented 1 year ago

@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 https://github.com/evanw/esbuild/issues/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

cutiful commented 1 year ago

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.

okikio commented 1 year ago

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

okikio commented 1 year ago

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

https://bundlejs.com/?q=%40chakra-ui%2Freact&treeshake=%5B%7B+ChakraProvider%2CContainer%2CInput+%7D%5D

CleanShot 2023-07-02 at 22 57 34@2x
okikio commented 1 year ago

@cutiful Does the fix work for you?

okikio commented 1 year ago

Since the fix seems to work ill close this issue

cutiful commented 1 year ago

@okikio sorry, I didn't have time to test. It does seem to be working on the main website now. Thanks again!

okikio commented 1 year ago

No worries, awesome to see that it's working again