-
-
Notifications
You must be signed in to change notification settings - Fork 270
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
Use lazy loading for routes #5914
Use lazy loading for routes #5914
Conversation
This reduces the main bundle size from 6.07 MB to 3.65 MB but increases overall application size from 14.04 MB to 14.18 MB. Most of the remainder of the main bundle size is from polyfills (2.35 MB) and libraries supporting localization (i18n). Signed-off-by: Taylor Smock <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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.
This is great! I'm impressed with the reduction in the initial page load size you achieved with your changes. I have a couple of curiosities regarding your approach:
I'm curious about your thought process behind merging similar chunks into one (like "CreateLicense," "ListLicenses," and "EditLicense" into a single "license" chunk). I understand that we could have generated separate chunks for each route, but I'm interested in understanding the rationale behind your choice.
Additionally, in the PR description, the build time for e11bc43 is 237.6 seconds, which seems unusually high compared to the 61.5 seconds I observed when generating the build. Could you provide insights on why there might be such a significant difference in build times?
I just split the routes into file-based chunks. Since (in your example) I think a bit more work needs to be done for that (
Best guess: I'm not just working on the TM, so it is possible that another project I'm working on takes enough resources that a run might be significantly longer than another. I did note that another run took ~77s for e11bc43. But it usually takes ~3 minutes on my system. Other possible reasons why there may have been a significant difference:
|
From the docs
We need webpack 5, which comes as part of #5908. I think I can add the magic comments before we update react-scripts, it just won't have any effect. EDIT: It looks like the magic comments aren't necessary. See webpack/webpack#16995. It just isn't documented yet. |
It doesn't look like splitting out additional chunks is currently possible; it might be possible with |
Thank you for the explanation. I appreciate your approach to merging similar chunks based on file structure. It makes sense to keep related routes together in the same chunk, and splitting out I must admit that I'm not well acquainted with these magic comments and webpack configurations myself, so your insights are valuable. |
Right now, we would have to move
Neither am I. I just did a lot of research. |
This reduces the amount of date that must be transmitted in order for the initial page (whatever it is) to load.
For the purposes of this analysis, I am using statoscope.tech with download speeds equivalent to fast 3g (1.4 MBit/s).
Note: Build time seems to be highly variable between runs; de65ce1 took 75.2s and e11bc43 took 76.9s in one run.
Initial chunks:
The top 3 dependencies in de65ce1 vendor packages are
refractor
,mapbox-gl
, anddate-fns
(~3.5 mb). I believe this size is without minification.The top 3 dependencies in e11bc43 vendor packages are
@sentry
,react-select
, and@formatjs
(~1.2 mb). Those are also in de65ce1.Both of the vendor packages are required for actual page content; it includes
react-dom
andreact-router
.The
main
chunk is largely made up of locales (~67% of the size is from locale data for e11bc43, and ~45% of the size is from locale data for de65ce1). Another PR should ensure we only load the locale(s) we need; this would reduce the amount of data sent for locales by 95% or more.