- 
                Notifications
    You must be signed in to change notification settings 
- Fork 7.5k
73% smaller app bundle, 1.5x faster time-to-interactive using dynamic imports #16426
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
base: master
Are you sure you want to change the base?
Conversation
| We can convert these into dynamic imports as well: 
 Overall bundle size reduction would be another 5 - 10%. | 
| Hi, thanks for your contribution! | 
| Exciting! Didn't you need to edit the webpack config too to make them standalone modules? | 
| Webpack seems to be correctly creating separate chunks for these automatically. I can see that they are also being loaded in the page when needed. However, just now realized I might need to edit the Makefile. webpack makes files like  | 
| Isn't there a way to set the chunk name? I thought a special type of comment could. | 
31cb9b8    to
    4ea6b97      
    Compare
  
    | @saghul Can you please take a look when you have the chance? | 
| Nice, thank you! Something we'll need to test is the impact of the changes on mobile, since the codebase is shared. | 
| <script><!--#include virtual="/config.js" --></script><!-- adapt to your needs, i.e. set hosts and bosh path --> | ||
| <script><!--#include virtual="/interface_config.js" --></script> | ||
| <script src="libs/lib-jitsi-meet.min.js?v=139"></script> | ||
| <script src="libs/react.min.js?v=139"></script> | 
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.
Doesn't this defeat the purpose of bundling it out, if we are going to load it at the start all the same?
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.
The react and vendor scripts are cached separately by the browser. So if we change our app code, browsers can reuse those (only app.bundle will be fetched).
We should only increment the ?v number for react and vendor if the package-lock.json file changes.
Stuff like excalidraw and rnnoise are still lazy loaded when needed.
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.
Fair point!
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.
@damencho Do you foresee a problem here? Any of our deployment scripts will need an update?
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.
Well here at build time we put the jitsi-meet new version and on our deployments we always use base url. So that will be updated with the packages. I don't see a reason to handle it differently than the app bundle.
There is no point of following package-lock file, as that is always changed between stable versions and so on.
| You're right, there are a few changes affecting the native implementation. I don't have the resources to build the app. Any ideas on how to test this? No linter/ts errors for web or native. | 
| It's possible you don't need to do anything because the Metro bundler will bundle them all the same, I was just pointing out a potential problem. | 
| Jenkins please test this please. | 
| Will it be possible to get this merged in before the next release? | 

Added dynamic imports for some of the largest libraries:
Bundle size change:
This results in significantly improved application performance across all key metrics, especially noticeable on slower network connections.
Fast Connection
Slow 4G Network (Real-World Conditions)
A live demonstration of the difference in loading times can be seen below:
Changes are divided into separate commits to make review easier.
Please let me know if I should update anything!