-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Add babel-plugin-optimize-react #32445
Conversation
5e51de6
to
9f89de3
Compare
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Webpack Runtime
App Entrypoints
Sections
Async-loaded Components
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. |
Thanks, @jsnajdr, this is a very interesting plugin! @trueadm: Thank you for listening to community feedback! 🙂If you have a chance to comment here, could you let us know whether the increase in the gzipped size is expected, or whether it's something that should be investigated? The increase in gzipped size seems unintuitive to me without a deeper understanding of what the plugin is doing. It's quite interesting that it's always one order of magnitude smaller than the reduction in parsed size, and roughly 10% of it. As it is, I'm not sure if the reduction in parsed size makes up for the increase in gzipped size. We'd be increasing network time in exchange for reducing CPU time, but it's not clear how impactful the latter would be without some in-depth testing. I'll be happy to look into that if the increase in gzipped size is indeed expected. |
Thanks for looking into this and doing this investigation! It's to be expected that gzip will suffer a bit, as there's duplication of In terms of what is better, it's very difficult to say. There's also the additional benefit of better theoretical runtime performance (as you're not doing as many object property lookups in the case of |
@trueadm No problem, I'll run some tests to find out! 👍 @jsnajdr Does running lab-assistant with "before" as the |
I don't know, probably yes 🙂 But I expect the performance benefit to be very small and hard to measure. Object property lookups and getter functions calls should be cheap operations. And there is so much other code being executed on every React render... |
With no throttling of any kind, there's no meaningful difference between both versions. With a 6x CPU throttle, I'd expect to see a benefit in CPU time if at all present, but there appears to be no significant change either. I'm currently modifying the tool to allow for network throttling and will report back. |
Throttling to 3G fast (1.6Mbps/768Kbps 150ms RTT) in addition to keeping the 6x CPU slowdown doesn't appear to produce any significant difference either. My testing appears to produce a standard deviation of around 1s across both sets of results, so I'm not surprised the tool isn't able to draw any meaningful conclusions from the data 😕 |
@sgomes Thanks for digging into it. Based on those results, I'd say that the extra build complexity of having this plugin is probably not going to give any real benefits to this app then. :) |
This was an experimental PR that can be closed now. Some of the optimizations that |
Trying out a Babel plugin I discovered when reading about Create React App 3.0 (facebook/create-react-app#6219).
The most interesting thing it does is reshuffling the React import code so that
React.createElement
references can be turned into one-letter mangled identifiers. This doesn't happen by default, at least not with webpack: accessing bindings imported from other modules is a rather complicated code that the minifier doesn't compress well.This should help compress large JSX trees, e.g., the inline SVGs we still use a lot.
The results are consisted across all our chunks: parsed size gets smaller, gzipped size gets bigger.
Cc @trueadm who expressed interest in large apps giving the plugin a test drive. See this bundle size report from out bot that runs
webpack-bundle-analyzer
on every PR.