[Maps] Code split Maps app #64594

thomasneirynck · 2020-04-27T21:57:38Z

Code-split the maps-plugin to reduce the initial maps.plugin.js size.

There are two main code dependencies in the plugin-init that are the root cause of the large bundle size.

Moves the load_layer_wizard dependency from the plugin-initialization to the GisMap component. GisMap wraps the entire application UX, including the add-layer-wizard. The layer wizards only need to be available there.
Lazy loads the dependencies of the Maps-embeddable. The MapEmbeddableFactory needs to be registered at plugin-initialization. However, this module imports a lot of core-application code because of its redux-selector dependencies. By code-splitting here, we avoid pulling in the entire Maps-app in the main bundle

This reduces the maps.plugin size from 5+MB to around 300kb.

$ ls -lh x-pack/plugins/maps/target/public/
total 75048
-rw-r--r--  1 thomasneirynck  staff   2.0M Apr 29 16:57 1.plugin.js
-rw-r--r--  1 thomasneirynck  staff   349K Apr 29 16:57 1990c3d0f490fa881871c5779ab8c212.js
-rw-r--r--  1 thomasneirynck  staff   1.8M Apr 29 16:57 2.plugin.js
-rw-r--r--  1 thomasneirynck  staff   178K Apr 29 16:57 3.plugin.js
-rw-r--r--  1 thomasneirynck  staff   220K Apr 29 16:57 4.plugin.js
-rw-r--r--  1 thomasneirynck  staff   299K Apr 29 16:57 40cd439cda90e7f7f4135b4037835e37.png
-rw-r--r--  1 thomasneirynck  staff   178K Apr 29 16:57 5.plugin.js
-rw-r--r--  1 thomasneirynck  staff   822K Apr 29 16:57 6.plugin.js
-rw-r--r--  1 thomasneirynck  staff    21K Apr 29 16:57 7.plugin.js
-rw-r--r--  1 thomasneirynck  staff   202K Apr 29 16:57 e3b1bd3256c6e2785c1c7786bf60a1a7.js
-rw-r--r--  1 thomasneirynck  staff   129K Apr 29 16:57 f01a207f57de9c098b15d34197acd75c.png
-rw-r--r--  1 thomasneirynck  staff   225K Apr 29 16:57 maps.plugin.js
-rw-r--r--  1 thomasneirynck  staff    30M Apr 29 16:57 stats.json

Not sure if we really need to preserve the <Suspense> loading of the React-components. I can leave it/remove.

todo:

fix TS type errors
functional test failure

~~WIP - just a POC~~

~~@restrry I'll need some guidance on where to exactly introduce this. You referred to the MapViewComponent~~
(

kibana/x-pack/plugins/maps/public/inspector/views/map_view.js

Line 12 in 5cab334

class MapViewComponent extends Component {

)
~~in #64317 (comment), but this is just the code for the inspector.~~

~~I introduced it a little closer to the actual map, but that isn't really making any dent in the bundle size.~~

elasticmachine · 2020-04-27T21:57:40Z

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

mshustov · 2020-04-28T08:24:24Z

I'll need some guidance on where to exactly introduce this. You referred to the MapViewComponent

Yeah, the rule of thumb is the higher Lazy component will be in the react tree the better.
With your changes, I still can see data plugin, @elastic/ems, @elastic/maki, d3 packages present in the initial bundle.
Also, I can see you register a bunch of Layer Wizards. Their React Components can be loaded asynchronously or you can alter renderWizard function to work with promises as well.

joshdover · 2020-04-28T22:50:55Z

Did not have much time today to try solutions, but I confirmed removing the import { registerLayerWizards } from './layers/load_layer_wizards'; import drops the maps bundle by ~60%.

I'm not sure when these need to be registered. Does this just need to be called before the embeddable renders and before the application mounts? If so, I think we can drop an async import in the embeddable rendering code.

thomasneirynck · 2020-04-29T13:46:08Z

I'm not sure when these need to be registered

Before the embeddable renders is sufficient, so it doesn't need to be in Plugin#start I think. I'll investigate.

…to maps/lazy_load

thomasneirynck · 2020-04-29T21:06:37Z

x-pack/plugins/maps/public/plugin.ts

 import { HomePublicPluginSetup } from '../../../../src/plugins/home/public';
 import { VisualizationsSetup } from '../../../../src/plugins/visualizations/public';
 import { MAP_SAVED_OBJECT_TYPE } from '../common/constants';
-import { MapEmbeddableFactory } from './embeddable';
+import { MapEmbeddableFactory } from './embeddable/map_embeddable_factory';


don't pull from the index.ts file as it brings in the actual Embeddable. Just pull in the factory.

x-pack/plugins/maps/public/embeddable/map_embeddable_factory.ts

kindsun · 2020-04-30T04:12:55Z

Pretty interesting stuff. Left some minor comments. Need to dig into this a bit more to understand some of the details.Thanks for taking this one on! 🤾‍♂️

nreese

Thanks for shrinking the maps plugin initial install size.

Can you remove the lazy load for MBMapContainer. Since this is used within GisMap, lazy loading GisMap will be sufficient.

Also can you remove gis_map/shim/index.ts and just change gis_map/index.ts to return default export

justinkambic · 2020-04-30T20:38:51Z

From the Uptime side I ran this locally and our map shows up like normal 👍

Not going to LGTM because it looks like the review/work for the feature is ongoing still.

rylnd · 2020-04-30T21:03:41Z

As of 231fb6d this is looking good on the SIEM side, but I'll double-check once this branch settles.

Nice bundle! 👍

thomasneirynck · 2020-05-03T23:37:48Z

@elasticmachine merge upstream

kibanamachine · 2020-05-04T01:34:22Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 37ea8ca

History

💔 Build #44653 failed 74551a5
💚 Build #44503 succeeded 231fb6d
💔 Build #44470 failed d39d2df
💚 Build #43586 succeeded 01e02b0

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

nreese

lgtm
code review, tested in chrome and firefox

rylnd

Checked this out again just now: from SIEM's perspective, things look/function as expected! 👍

joshdover

This looks great! I don't see any more obvious opportunities to split anything else out of the main bundle (which is now only 46kb gzipped!)

justinkambic

For Uptime, the map still LGTM 👍

Code-split the maps-plugin to reduce the initial `maps.plugin.js` size. There were two main code dependencies in the plugin initialization that were the root cause of the large bundle size. - `GisMap` wraps the entire application UX, including the add-layer-wizard. The layer wizards only need to be available there. This PR moves the `load_layer_wizard` dependency from the plugin-initialization to the `GisMap` component. - The `MapEmbeddableFactory` needs to be registered at plugin-initialization. However, this module imports a lot of core-application code. By code-splitting here, we avoid pulling in the entire Maps-app in the main bundle. This also introduces a lazy-initialization of the `GisMap` itself as an additional split to further reduce size of the bundles.

* [Maps] Code split Maps app (#64594) Code-split the maps-plugin to reduce the initial `maps.plugin.js` size. There were two main code dependencies in the plugin initialization that were the root cause of the large bundle size. - `GisMap` wraps the entire application UX, including the add-layer-wizard. The layer wizards only need to be available there. This PR moves the `load_layer_wizard` dependency from the plugin-initialization to the `GisMap` component. - The `MapEmbeddableFactory` needs to be registered at plugin-initialization. However, this module imports a lot of core-application code. By code-splitting here, we avoid pulling in the entire Maps-app in the main bundle. This also introduces a lazy-initialization of the `GisMap` itself as an additional split to further reduce size of the bundles. * fix merge Co-authored-by: Elastic Machine <[email protected]>

nreese and others added 3 commits April 27, 2020 12:41

[Maps] do not display EMS or kibana layer wizards when not configured

5aed5dd

default tilemap to empty object instead of array

f0ae4af

lazy load mb container

f559c26

thomasneirynck added chore WIP Work in progress [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.8.0 labels Apr 27, 2020

thomasneirynck added 2 commits April 27, 2020 18:27

lady load embeddable

a23bfff

rename

01e02b0

nreese and others added 6 commits April 29, 2020 08:35

Merge branch 'master' of github.com:elastic/kibana into check_visiblity

4f09caa

move registration

1a95199

Merge branch 'check_visiblity' of https://github.com/nreese/kibana in…

3cfba6b

…to maps/lazy_load

tmp

56ae2ac

Merge branch 'master' into maps/lazy_load

c4bc4ae

split on factory

1b64dae

thomasneirynck commented Apr 29, 2020

View reviewed changes

remove some cruft

8f881a6

thomasneirynck changed the title ~~[Maps] POC lazy load container~~ [Maps] Code split Maps app Apr 29, 2020

no need to code-split on registration

4ea8906

thomasneirynck marked this pull request as ready for review April 29, 2020 21:25

thomasneirynck requested a review from a team as a code owner April 29, 2020 21:25

thomasneirynck requested review from mshustov, joshdover, nreese and kindsun April 29, 2020 21:25

kindsun reviewed Apr 30, 2020

View reviewed changes

x-pack/plugins/maps/public/embeddable/map_embeddable_factory.ts Outdated Show resolved Hide resolved

thomasneirynck added 4 commits April 30, 2020 11:00

no need to delay constant loading

64175f1

review

2d66a39

remove for consistency

d39d2df

add typing

231fb6d

thomasneirynck requested a review from a team April 30, 2020 19:37

nreese reviewed Apr 30, 2020

View reviewed changes

thomasneirynck requested a review from justinkambic April 30, 2020 20:23

Merge branch 'master' of github.com:elastic/kibana into maps/lazy_load

ed3a9b8

thomasneirynck added 3 commits April 30, 2020 17:07

remove lazy mapcontainer

79da694

remove shim

54c875d

add some comments

74551a5

thomasneirynck requested review from nreese and kindsun April 30, 2020 22:08

Merge branch 'master' into maps/lazy_load

37ea8ca

nreese approved these changes May 4, 2020

View reviewed changes

thomasneirynck removed the WIP Work in progress label May 4, 2020

rylnd approved these changes May 4, 2020

View reviewed changes

joshdover approved these changes May 4, 2020

View reviewed changes

justinkambic approved these changes May 4, 2020

View reviewed changes

thomasneirynck merged commit 7bf7174 into elastic:master May 4, 2020

thomasneirynck mentioned this pull request May 4, 2020

[7.x] [Maps] Code split Maps app (#64594) #65178

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Maps] Code split Maps app #64594

[Maps] Code split Maps app #64594

thomasneirynck commented Apr 27, 2020 •

edited

Loading

elasticmachine commented Apr 27, 2020

mshustov commented Apr 28, 2020 •

edited

Loading

joshdover commented Apr 28, 2020

thomasneirynck commented Apr 29, 2020

thomasneirynck Apr 29, 2020

kindsun commented Apr 30, 2020

nreese left a comment

justinkambic commented Apr 30, 2020

rylnd commented Apr 30, 2020

thomasneirynck commented May 3, 2020

kibanamachine commented May 4, 2020

nreese left a comment

rylnd left a comment

joshdover left a comment

justinkambic left a comment

[Maps] Code split Maps app #64594

[Maps] Code split Maps app #64594

Conversation

thomasneirynck commented Apr 27, 2020 • edited Loading

elasticmachine commented Apr 27, 2020

mshustov commented Apr 28, 2020 • edited Loading

joshdover commented Apr 28, 2020

thomasneirynck commented Apr 29, 2020

thomasneirynck Apr 29, 2020

Choose a reason for hiding this comment

kindsun commented Apr 30, 2020

nreese left a comment

Choose a reason for hiding this comment

justinkambic commented Apr 30, 2020

rylnd commented Apr 30, 2020

thomasneirynck commented May 3, 2020

kibanamachine commented May 4, 2020

💚 Build Succeeded

History

nreese left a comment

Choose a reason for hiding this comment

rylnd left a comment

Choose a reason for hiding this comment

joshdover left a comment

Choose a reason for hiding this comment

justinkambic left a comment

Choose a reason for hiding this comment

thomasneirynck commented Apr 27, 2020 •

edited

Loading

mshustov commented Apr 28, 2020 •

edited

Loading