Skip to content
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

Lazy loading React components #64179

Closed
mshustov opened this issue Apr 22, 2020 · 9 comments
Closed

Lazy loading React components #64179

mshustov opened this issue Apr 22, 2020 · 9 comments
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation discuss performance Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team

Comments

@mshustov
Copy link
Contributor

mshustov commented Apr 22, 2020

I created this issue to discuss the lazy loading strategy for react components used by different registries in Kibana.
If you build Kibana platform plugins locally and check their sizes. You can notice that maps plugin size reaches 4.8Mb.
That's mostly due to new optimizer architecture including all the plugin dependencies in the bundle and lack of code tree-shaking (WIP #62390). Even if we move some deps to shared dependencies and remove unused code, the build size will be quite significant (@elastic/ems-client, @elastic/maki only gives us 1Mb)
Ideally, a plugin code should be as slim as possible and include the thing that necessary for proper registration. All other code must be loaded on demand.

I'm wondering if we can improve the situation with loading Maps React components on demand.
To understand how it affects the initial load time let's look at maps.plugin.js size (this is the chunk loaded every time a user opens the Kibana):
without lazy loading - 4.8Mb
with MapViewComponent loaded lazily - 29Kb

There are several approaches to address the problem:

Maybe @elastic/kibana-app-arch has other ideas or opinion of whatever pattern is preferable?

@mshustov mshustov added discuss Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc performance [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation Team:AppArch labels Apr 22, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@kibanamachine kibanamachine added this to To triage in kibana-app-arch Apr 22, 2020
@mshustov
Copy link
Contributor Author

We have the same problem for visualizations registry as well:

  • vis_type_timeseries plugin - 4Mb
  • vis_type_markdown plugin - 2.8Mb
  • vis_type_metric plugin - 2.4Mb

@mshustov mshustov added the Team:Operations Team label for Operations Team label Apr 22, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@Dosant
Copy link
Contributor

Dosant commented Apr 22, 2020

Wonder if React.lazy and Suspense are working in Kibana env? If yes, I guess we won't need react-loadable?

@mshustov
Copy link
Contributor Author

mshustov commented Apr 23, 2020

@Dosant yes. I did try it in our app and all integration tests passed.
#64285
Regarding the other infrastructure:

I think you can use React.Lazy if you want.
Do you want me to create a separate issue for @elastic/kibana-app-arch team to refactor visualizations plugins?
That would be great if we can have it fixed until FF v7.8. There are some complaints about Kibana loading time.

@Dosant
Copy link
Contributor

Dosant commented Apr 23, 2020

@restrry, thanks for checking react.lazy 👍
I guess which team fixes it depends on where we do lazy loading. If lazy loading will be handled inside each visualisation type by it's own - then I guess it would be @elastic/kibana-app. But if we somehow make it part of visualisations infrastructure, then @elastic/kibana-app-arch.
I'll bring in up to the team

@timroes
Copy link
Contributor

timroes commented Apr 23, 2020

Related to #58280

@Dosant
Copy link
Contributor

Dosant commented Apr 23, 2020

@restrry, @elastic/kibana-app-arch is going to look into generic approach for code splitting of app arch registries and components during our next planning week.
In a meantime, if possible during 7.8, we suggest @elastic/kibana-app to look into specific visualisations implementations and wrap them with React.Lazy where it would benefit the most.

kibana-app-arch automation moved this from To triage to Done in current release May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation discuss performance Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Operations Team label for Operations Team
Projects
kibana-app-arch
  
Done in current release
Development

No branches or pull requests

4 participants