document code splitting for client code #62593
Conversation
mshustov
added
chore
Team:Core
|
Pinging @elastic/kibana-platform (Team:Platform) |
| return mountApp(await core.getStartServices(), params); | ||
| }, | ||
| }); | ||
| plugins.management.sections.getSection('another').registerApp({ |
There was a problem hiding this comment.
NIT: this returns Section | undefined so the example would need a getSection('another')!. to be totally exact, but we probably don't care for the role of the example.
| Everyone loves snappy applications with responsive UI and hates spinners. Users deserve the best user experiences regardless of whether they run Kibana locally or in the cloud, regardless of their hardware & environment. | ||
| There are 2 main aspects of the perceived speed of an application: loading time and responsiveness to user actions. | ||
| New platform loads and bootstraps **all** the plugins whenever a user lands on any page. It means that adding every new application affects overall **loading performance** in the new platform, as plugin code is loaded **eagerly** to initialize the plugin and provide plugin API to dependent plugins. | ||
| However, it's usually not necessary that the whole plugin code should be loaded and initialized at once. The plugin could keep on loading code covering API functionality on Kibana bootstrap but load UI related code lazily on-demand, when an application page or management section is mounted. |
There was a problem hiding this comment.
I would maybe insist on the fact that even if UI components size may be minimal, excluding/splitting them from the initial bundle also excludes all UI-related dependencies such as react and react-dom. UI related deps are almost always > 2.5mb uncompressed from what I saw.
There was a problem hiding this comment.
I would maybe insist on the fact that even if UI components size may be minimal
do you mean something like always prefer to load UI components lazily, even if UI components size seems to be negligible, because it requires loading of some rendering tools?
initial bundle also excludes all UI-related dependencies such as react and react-dom
react & react-dom are already excluded from a bundle as provided via kbn-ui-shared-deps.
There was a problem hiding this comment.
Probably not the best wording, but the idea would be always prefer to require UI root components lazily when possible (such as in mount handlers). Even if their size may seem negligible, it's very likely that they are using some heavy-weight libraries that will also be removed from the initial plugin bundle therefor reducing its size by a significant amount
react & react-dom are already excluded from a bundle as provided via kbn-ui-shared-deps.
From #62263 (comment) I think kbn-ui-shared-deps is used only by legacy plugins? Even if not, there seems to be other heavy-weight dependencies that are imported mostly/only from UI components as cutting them the the main bundle reduces its size by a lot.
| ## Keep Kibana fast | ||
| **tl;dr**: Load as much code lazily as possible. | ||
| Everyone loves snappy applications with responsive UI and hates spinners. Users deserve the best user experiences regardless of whether they run Kibana locally or in the cloud, regardless of their hardware & environment. |
There was a problem hiding this comment.
It's outside of code splitting (well, of splitting plugin bundles), but another thing that is currently increasing our bundles size drastically are static imports that can be done between plugins (at least until #62390 is merged)
See 43a9d45 for example, where even with async mount method for the route mount function, all dependencies were kept because of a static import from another plugin that was using them (in addition to shipping the whole dependency plugin).
Not sure if it belongs there or more in our CONVENTION, but we should probably also declare that static exports from plugins should be avoided as much as possible and exposed from the plugins contracts instead. This ensure both that plugins will not import more that they should from other plugins and also reduce the overall initial load, as the 'static' code will only be shipped in the providing plugin code.
There was a problem hiding this comment.
but we should probably also declare that static exports from plugins should be avoided as much as possible and exposed from the plugins contracts instead
I do agree it's a problem: a plugin importing anything from the data plugin will get additional (mostly unnecessary) 1.4Mb of code.
But I'd argue we shouldn't ban of static imports usage. Using static imports is a valid use-case for sharing stateless code. If we forced plugins to use plugins contracts only, it would worsen DX because it increases the number of dependencies passed down the code. I'd rather say the current situation is a technical limitation, that shouldn't dictate plugin authors how to write their code and platform & operation teams have to fix it eventually.
There was a problem hiding this comment.
After thinking a little about it you are right imho, this is only driven by a technical limitation that should be addressed, and is not a bad practice on itself.
| - [an official tool](http://webpack.github.io/analyse/#modules) from webpack authors | ||
| - [webpack-visualizer](https://chrisbateman.github.io/webpack-visualizer/) |
There was a problem hiding this comment.
I personally have been using webpack-bundle-analyzer https://www.npmjs.com/package/webpack-bundle-analyzer on it's CLI version (webpack-bundle-analyzer bundle/output/path/stats.json). not sure if we want to be exhaustive here though.
|
Just a heads up - we're automatically skipping CI on doc only changes so |
* add lazy loading section for client bundles * add Pierres suggestion
* master: (132 commits) document code splitting for client code (elastic#62593) Escape single quotes surrounded by double quotes (elastic#63229) [Endpoint] Update cli mapping to match endpoint package (elastic#63372) update in-app links to metricbeat configuration docs (elastic#63295) investigation notes field (documentation / metadata) (elastic#63386) [Maps] fix bug where toggling Scaling type does not re-fetch data (elastic#63326) [Alerting] set correct parameter for unauthented email action (elastic#63086) [Telemetry] force staging urls in tests (elastic#63356) Migrate legacy maps service to NP & update refs (elastic#60942) Fix task manager query to return tasks to retry (elastic#63360) [Endpoint] Policy list support for URL pagination state (elastic#63291) [Canvas] Migrate saved object mappings and migrations to Kibana Platform (elastic#58891) [DOCS] Add ILM tutorial (elastic#59502) [Maps] Add SOURCE_TYPES enumeration (elastic#62975) [Maps] update geospatial filters to use geo_shape query for geo_point fields (elastic#62966) Move away from npStart for embeddables in canvas (elastic#62680) Use MapInput type from Maps plugin (elastic#61539) Update to pagination for workpad and templates (elastic#62050) [SIEM] Fix AlertsTable id (elastic#63368) Consistent terminology around cypress test data (elastic#63279) ...
* master: document code splitting for client code (elastic#62593) Escape single quotes surrounded by double quotes (elastic#63229) [Endpoint] Update cli mapping to match endpoint package (elastic#63372) update in-app links to metricbeat configuration docs (elastic#63295) investigation notes field (documentation / metadata) (elastic#63386) [Maps] fix bug where toggling Scaling type does not re-fetch data (elastic#63326) [Alerting] set correct parameter for unauthented email action (elastic#63086) [Telemetry] force staging urls in tests (elastic#63356) Migrate legacy maps service to NP & update refs (elastic#60942) Fix task manager query to return tasks to retry (elastic#63360) [Endpoint] Policy list support for URL pagination state (elastic#63291) [Canvas] Migrate saved object mappings and migrations to Kibana Platform (elastic#58891) [DOCS] Add ILM tutorial (elastic#59502) [Maps] Add SOURCE_TYPES enumeration (elastic#62975) [Maps] update geospatial filters to use geo_shape query for geo_point fields (elastic#62966) Move away from npStart for embeddables in canvas (elastic#62680)
* alerting/alert-services-mock: (107 commits) removed unused import added alert services mock and use it in siem [Metrics UI] Refactor With* containers to hooks (elastic#59503) [NP] Migrate logstash server side code to NP (elastic#63135) Clicking cancel in saved query save modal doesn't close it (elastic#62774) [Lens] Migration from 7.7 (elastic#62879) [Lens] Fix bug where suggestions didn't use filters (elastic#63293) Task/linux events (elastic#63400) [Remote clusters] guard against usageCollection plugin if unav… (elastic#63284) [Uptime] Remove pings graphql (elastic#59392) Index Pattern Field class - factor out copy_field code for future typescripting (elastic#63083) [EPM] add/remove package in package settings page (elastic#63389) Adjust API authorization logging (elastic#63350) Revert FTR: add chromium-based Edge browser support (elastic#61684) (elastic#63448) [Event Log] Adds namespace into save objects (elastic#62974) document code splitting for client code (elastic#62593) Escape single quotes surrounded by double quotes (elastic#63229) [Endpoint] Update cli mapping to match endpoint package (elastic#63372) update in-app links to metricbeat configuration docs (elastic#63295) investigation notes field (documentation / metadata) (elastic#63386) ...
* add lazy loading section for client bundles * add Pierres suggestion
[skip-ci]