-
Notifications
You must be signed in to change notification settings - Fork 2k
Update terser to 5.2.1 #45211
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
Update terser to 5.2.1 #45211
Conversation
Also update `terser-webpack-plugin` to enable using the new `terser`.
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: App Entrypoints (~3613 bytes removed 📉 [gzipped]) Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used. Sections (~10570 bytes removed 📉 [gzipped]) Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Async-loaded Components (~2110 bytes removed 📉 [gzipped]) React components that are loaded lazily, when a certain part of UI is displayed for the first time. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
|
Looks like Terser somehow got worse, and now produces larger output 😞 |
|
Note: the output is now actually smaller than before, since I enabled 2 passes on the compression. This only appears to add ~ 30s build time on the |
@sgomes When I'm looking at the original ICFY comment, before it was updated after I'd like to reproduce the "output got larger" problem. |
|
@jsnajdr: The original comment has everything getting larger in gzipped size except for some async chunks. If you check the original comment in the history it's still there. E.g.: There are a lot of chunks getting smaller in parsed size but larger in gzipped size; I'm not sure if that's real or an artifact of how the bundle analyzer measures things. If you want to reproduce locally, you can try commit f2d8838. |
|
@sgomes Thanks for confirming! I was curious what part of |
Oh, that's good data, thank you @jsnajdr! 👍 |
|
Did some Terser learning today and I have some findings why we see the changes we do. Suppose we have code like this: s.getItem("jetpack_cloud_redirect_path_expires_in");
s.setItem("jetpack_cloud_redirect_path_expires_in");Since 5.2.0, Terser has a slightly different criteria whether to extract or inline variables and newly transforms this code into: let k = "jetpack_cloud_redirect_path_expires_in";
s.getItem(k);
s.setItem(k);This makes the unzipped code shorter, but steals come compression opportunities from the gzip algorithm. The result is that the gzipped size is a few bytes bigger. This gzip paradox is not new or rare: last year, we were experimenting with a webpack plugin that would optimize the What benefits do the two passes give us:
var a = 1; var b = 2;
// into
var a = 1, b = 2;
a(1);function b() { return 1; };
// into
function b() { return 1; }a(1);Apparently, some of these opportunities are created only after other code transforms are done: they were not in the original code. |
jsnajdr
left a comment
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.
Looks ready to 🚢
I'm not excited about the passes: 2 change. Build times and occasional Terser OOM errors are a big problem for us. On the other hand, I'd argue that adding a few bytes to the output, where the addition is a rather random coincidence, is not a that big problem 🙂
Yeah, I totally see where you're coming from. The gains are quite good on some chunks, though (2.7KB gzipped), so I think we should give it a fair shot. If |
|
Thank you for the review, @jsnajdr! 👍 |
Optional chaining will start making its way into the minification phase of the pipeline once Safari 14 ships (since this will mean all browsers in the
evergreenbuild support optional chaining). Unfortunately, the current version of Terser we use does not support this syntax and will start failing.This PR updates to the latest version of Terser, so that we're ready ahead of time.
Changes proposed in this Pull Request
terserto 5.2.1terser-webpack-pluginto enable using the newterser.Testing instructions
Smoke-test the live branch and ensure that the new Terser doesn't introduce any unexpected breakage.