-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Let terser eliminate transpiled classes #3611
Conversation
Benchmark Resultspackages/benchmarks/kitchen-sink
Timings
Cold Bundles
Cached Bundles
packages/benchmarks/react-hn
Timings
Cold BundlesNo bundle changes detected. Cached BundlesNo bundles found, this is probably a failed build... |
@@ -12,7 +12,7 @@ export function generate( | |||
) { | |||
let {code} = babelGenerate(ast, { | |||
minified: options.minify, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mischnic what does this tell babel to do? Should it also just be left to terser?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT, it only compresses whitespaces:
var generate = require("@babel/generator").default
var parse = require("@babel/parser").parse
const AST = parse(`const ABCDEF = "hel" + "lo";`);
console.log(generate(AST, {minified: true}).code);
// const ABCDEF="hel"+"lo";
↪️ Pull Request
A statement like this doesn't get tree shaken because it does look pure (without looking at the comment).
By outputting comments when generating the code after scopehoisting (and before terser), we can leverage terser to remove everything.
This also makes it possible to allow certain comments to be preserved by terser (e.g. license headers): fixes #3322
We could a handle these comments in
treeShake
, but I'm not sure if there is an advantage to doing that?+
Smaller cache size, less data to serialize (both to cache and AST->code)-
larger error surface, would need more tests(it might be more complicated than that: rollup/rollup#2429)