[Question] Tree shaking #50
Comments
|
Yes, I'm planning on doing this, but it'll likely be after other features land first. |
|
Note to self: There's a tree shaking size comparison here that could be interesting: #86 (comment). |
|
Another example: #81 (comment) |
|
Thanks for these links! These are very helpful. I have tree shaking basically implemented, and about to be released. It's currently in a branch as I test it and prepare it for release. This release is going to be about correctness, not minimal bundle sizes. It introduces the tree shaking data structure but doesn't yet handle the side-effect annotations you mention in #86 (comment). That will have to come in subsequent releases. Is there information anywhere about how the |
|
Just as one would find the defacto pure annotations test examples in the uglify-js tree where it was first implemented, you'd have to look at the webpack source tree for "sideEffects" test cases. The last link in #86 (comment) was actually the official documentation for the latter. |
|
Yeah I'll have to study the test cases, and do some reverse engineering to answer some of my questions. I read the documentation but it was too high-level and didn't really tell me enough information to implement it. I just released version 0.4.0 which has the preliminary tree shaking support described above. Please try it out and let me know if you encounter any correctness issues. As I said above, esbuild isn't respecting annotations yet so the size is likely still going to be bigger than other bundlers. Right now tree shaking is only based on which top-level statements reference which other top-level statements. I'll do another push later on to improve on bundle sizes. |
|
The
If a side effect free file within a module is imported and no symbols are used from it, then the import can be dropped altogether. But if any exported symbol is imported from a side effect free file then its top level statements with side effects must be retained, and only its unreferenced symbols without side effects may be dropped. To use a garbage collector analogy, any used exports or top level statements with side effects are considered to be roots. Here's an example with additional documentation: https://github.com/webpack/webpack/tree/master/examples/side-effects |
|
The following was observed behavior from rollup's implementation:
but it doesn't seem to be logically consistent with I don't know whether webpack has the same behavior as rollup with an empty |
|
🎉🎉🎉 Upgrading serverless-esbuild |
Yes, that model makes sense to me. That's the trivial way to do things safely.
This is what I've implemented in the latest release.
Since code is JavaScript, it's impossible to determine what is guaranteed to be free of side effects outside of a whitelist of a few types of expressions such as literals. I was wondering if the export function foo() {}
foo.bar = 123It's impossible to determine based on this code whether the second statement has side effects or not. Someone might have done this somewhere before this was evaluated: Object.defineProperty(Function.prototype, 'bar', {
set() {
sideEffects()
}
})And if the second statement is retained then the first line must also be retained. I'm wondering if I could see an alternative algorithm where, if |
The side effect determination ought be the same whether or not Be aware that Rollup and Terser can be fooled by pathological cases of |
|
For people following along, there's a thread relevant to tree shaking in #86. Let's merge that thread into this one. I have cloned https://github.com/mischnic/tree-shaking-example into https://github.com/evanw/tree-shaking-example and added support for I just released esbuild 0.4.6 with support for The jump from 0.3.9 to 0.4.2 is the initial tree shaking support for ES6 imports and exports. This improves various benchmarks including The jump from 0.4.2 to 0.4.6 is the newly-released support for the Follow-up work is support for |
|
Please update to the latest versions of parcel, rollup, webpack, babel and related packages: --- a/package.json
+++ b/package.json
@@ -20,22 +20,21 @@
"rxjs": "^6.5.2"
},
"devDependencies": {
- "@babel/core": "^7.4.4",
- "@babel/preset-env": "^7.4.4",
- "babel-loader": "^8.0.6",
+ "@babel/core": "7.10.2",
+ "@babel/preset-env": "7.10.1",
+ "babel-loader": "8.1.0",
"console.table": "^0.10.0",
"filesize": "^4.1.2",
"npm-run-all": "^4.1.5",
- "nyc": "^14.1.1",
- "parcel-bundler": "^1.12.3",
- "rollup": "^1.12.1",
- "rollup-plugin-babel": "^4.3.2",
- "rollup-plugin-commonjs": "^10.0.0",
- "rollup-plugin-node-resolve": "^5.0.0",
- "rollup-plugin-terser": "^4.0.4",
- "terser-webpack-plugin": "^1.2.4",
- "webpack": "^4.31.0",
- "webpack-cli": "^3.3.2"
+ "parcel-bundler": "1.12.4",
+ "rollup": "2.13.1",
+ "rollup-plugin-babel": "4.4.0",
+ "rollup-pluginutils": "2.8.2",
+ "rollup-plugin-commonjs": "10.1.0",
+ "rollup-plugin-node-resolve": "5.2.0",
+ "rollup-plugin-terser": "6.1.0",
+ "webpack": "4.43.0",
+ "webpack-cli": "3.3.11"
},
"scripts": {
"clean": "rm -rf .cache parcel rollup webpack esbuild-*",
@@ -55,7 +54,7 @@
"esbuild:rambda": "LIB='rambda' node esbuild.js",
"esbuild:ramda": "LIB='ramda' node esbuild.js",
"esbuild:ramdaBabel": "LIB='ramdaBabel' node esbuild.js",
- "parcel:lodash": "parcel build src/lodash.js --experimental-scope-hoisting -d parcel",
+ "parcel:lodash": "parcel build src/lodash.js -d parcel",
"parcel:lodash-es": "parcel build src/lodash-es.js --experimental-scope-hoisting -d parcel",
"parcel:rxjs": "parcel build src/rxjs.js --experimental-scope-hoisting -d parcel",
"parcel:react-icons": "parcel build src/react-icons.js --experimental-scope-hoisting -d parcel",
--- a/rollup.config.js
+++ b/rollup.config.js
@@ -27,5 +27,4 @@ export default [
}
},
- sourcemap: false,
toplevel: true
})
Might be a parcel scope hoisting bug. Disabling @mischnic could provide a better answer.
CJS require/exports and ES import/export constructs should not be mixed within the same source file. This works for all bundlers: --- a/src/ramdaBabel.js
+++ b/src/ramdaBabel.js
@@ -13,4 +13,4 @@ function fn(x) {
)(x);
}
-export const answer = fn(10).join(',');
+exports.answer = fn(10).join(','); |
|
Thanks for the corrections. I just applied them to https://github.com/evanw/tree-shaking-example. I also fixed some bugs I recently found with Here's the latest run with version 0.4.7 of esbuild: The bundle size from esbuild is now competitive with every benchmark except for |
It's legit to alter an export property. There's nothing unsafe about that: The unreferenced icons can safely be dropped. |
|
This is what I was posting about above: #50 (comment). Those assignments may have side effects. For example, this code may have been run before that code was executed: Object.defineProperty(Function.prototype, 'displayName', {
set() {
sideEffects()
}
}) |
No bundler or minifier would assume builtins would be altered with pathological setters. esbuild can safely assume the same. |
|
Library or application code is fundamentally different than polyfills. Generally polyfills are not run through bundlers, and if so they would be run without code optimization. |
I wouldn't trust Parcel 1's tree shaking. Parcel 2 works correctly here. Looks like Parcel 2 produces a bigger bundle for "remeda" than Parcel 1, I'll have to look into that. I've added another testcase for a known Parcel deopt (parcel-bundler/parcel#4565), where esbuild does very well (even compared to rollup): |
|
@mischnic - Thanks for your insight and adding the new test case. I thought I had upgraded all the bundlers but I didn't realize that parcel v2 hasn't been officially released yet. I wonder what happened to rollup/react-icons between https://github.com/evanw/tree-shaking-example: and the newly revised https://github.com/mischnic/tree-shaking-example: It may be due to the addition of the rollup namedExport for "react-is" to support the material-ui test case. Edit: I see that rollup-plugin-node-polyfills was also added to the Rollup config. Does react offer an ES version of their library? This UMD/CJS bundling is less than ideal. /cc @lukastaegert |
That was causing it. I've change the config to only run that plugin for material-ui. |
|
Strange about needing rollup-plugin-node-polyfills for the Rollup material-ui test case. Here's a comparable material-ui example app that doesn't require it to build: #81 (comment) |
|
|
In an effort to make an apples to apples comparison, do the other bundlers need a node polyfill for the material-ui test case to operate on a NodeJS target? |
|
Parcel and Webpack include them automatically by default |
esbuild doesn't, afaik. |
|
Actually, the Rollup doesn't respect Edit: I've fixed the Rollup config, now Parcel is the only outlier in that benchmark. |
|
Thanks again @mischnic. I was unaware of this rollup plugin functionality: plugins: [
- resolve(),
+ resolve({
+ browser: true
+ }),Parcel and esbuild reducing configuration complexity is certainly the way to go. In my opinion a lot of Rollup's plugins should be built in and automatically configured. |
|
FYI: support for tree shaking of |
|
Nice. Other reacty things could have pure annotations as well - see babel/babel#11428. Side note... it would be useful if Rollup supports bundling from stdin: All imports would be relative to the current working directory when the entry point is stdin. Off topic, but somewhat related... react doesn't appear to have |
This works in esbuild as of version 0.6.1. |
|
I am not sure but it looks like Realms proposal could potentially help with ckecking if class A {}
A.x = 1;is side-effect free, e.g. when entire bundle is wrapped in a Realm. |
|
A contemporary option would be |
|
Hey folks, any idea how to debug tree shaking? Is there a verbose mode that could tell when tree-shaking is disabled (detected a side-effect, incorrectly set main fields etc.)? It's quite difficult to tell if tree shaking is working ok or not on a massive codebase, and which module or package maybe faulty for dead code remaining. For instance, I end up with many unused variables in my bundled script. I use Esbuild via Tsup if that changes anything. |
@evanw, are you planning to add tree shaking to exclude unused code from bundles?
The text was updated successfully, but these errors were encountered: