[Visualize] Lazy load default editor, fix duplicated styles by sulemanof · Pull Request #66732 · elastic/kibana

sulemanof · 2020-05-15T14:02:52Z

Summary

This is intended to split loading the default editor code and prevent duplicating of css rules across all places where stuff is imported from vis_default_editor/public (the most of vis types)

The EuiLoadingChart mono is added as spinner (more visible if internet is slow):

And the EuiErrorBoundary if something went wrong:

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

src/plugins/vis_default_editor/public/default_editor_controller.tsx

flash1293 · 2020-05-18T12:45:41Z

I checked this and it only brings the initial bundle from 1.3M to 1.2M. One quick win would be to not import the default editor within the initial bundle (20% contributor):

src/plugins/visualize/public/kibana_services.ts
37:import { DefaultEditorController } from '../../vis_default_editor/public';

src/plugins/visualize/public/plugin.ts
43:import { DefaultEditorController } from '../../vis_default_editor/public';

For some reason, currently the stateless "DefaultEditorController" is put into the services object directly in plugin.ts. It looks like it would be totally fine to just import the default controller in src/plugins/visualize/public/application/editor/visualization_editor.js where it's used. It looks like this is also the only way lodash is introduced into the initial bundle (another 20% contributor), but maybe it's also imported from somewhere else, not sure.

Also, 30% of the initial bundle is caused by core, probably because of the DEFAULT_APP_CATEGORIES import in plugin.ts. @restrry are there plans to avoid this or something non-hacky we can do from our side?

mshustov · 2020-05-18T15:14:59Z

Also, 30% of the initial bundle is caused by core, probably because of the DEFAULT_APP_CATEGORIES import in plugin.ts. @restrry are there plans to avoid this or something non-hacky we can do from our side?

@flash1293 we are still planning to land tree-shaking in 7.9 (and hopefully we can port it to 7.8)

flash1293 · 2020-05-18T15:36:58Z

@restrry Thanks, makes sense. I'm really excited for this :)

@sulemanof I think we can reap the vis_default_editor benefits now, but anything else can probably be postponed till after tree shaking lands.

elasticmachine · 2020-05-19T13:17:48Z

Pinging @elastic/kibana-app (Team:KibanaApp)

flash1293 · 2020-05-19T13:59:14Z

Down to 837K 💯 Almost all of the rest of the bundle is core being pulled in which should be fixed by tree shaking.

flash1293

Tested in Firefox, Visualize still works, LGTM.

kibanamachine · 2020-05-19T15:22:11Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 130dba3

History

💚 Build #48592 succeeded 68c9de1
💔 Build #48348 failed 7763e29
💔 Build #48313 failed 0cfda13

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

cchaos

I didn't test locally, but the SASS imports look ok.

…66732) * Lazy load bundle, fix duplicated styles * Add EuiLoadingChart and EuiErrorBoundary * Return localStorage service * Use mono spinner * Get rid of loading the editor outside of visualize mounting # Conflicts: # src/plugins/visualize/public/kibana_services.ts # src/plugins/visualize/public/plugin.ts

…67116) * Lazy load bundle, fix duplicated styles * Add EuiLoadingChart and EuiErrorBoundary * Return localStorage service * Use mono spinner * Get rid of loading the editor outside of visualize mounting # Conflicts: # src/plugins/visualize/public/kibana_services.ts # src/plugins/visualize/public/plugin.ts

…67115) * Lazy load bundle, fix duplicated styles * Add EuiLoadingChart and EuiErrorBoundary * Return localStorage service * Use mono spinner * Get rid of loading the editor outside of visualize mounting

…ent/add-support-in-url-for-hidden-toggle * 'master' of github.com:elastic/kibana: (49 commits) [Uptime] Improve responsiveness details page (elastic#67034) skip flaky suite (elastic#66669) Revert "Integration of a static filesystem for the node_modules (elastic#47998)" (elastic#67124) Support api_integration/kibana/stats against remote hosts (elastic#53000) chore(NA): add module name mapper for src plugins on x-pack (elastic#67103) Change the error message on TSVB in order to be more user friendly (elastic#67090) [kbn/optimizer] poll parent process to avoid zombie processes (elastic#67059) [Visualize] Lazy load default editor, fix duplicated styles (elastic#66732) Bump styled-component dependencies (elastic#66611) Bump react-markdown dependencies (elastic#66615) Fix Core docs links (elastic#66977) Timelion graph is not refreshing content after searching or filtering (elastic#67023) Remove `--xpack.endpoint.enabled=true` from README.md file (elastic#67053) Move apm tutorial from apm plugin into apm_oss plugin (elastic#66432) [Logs UI] Restore call to `UsageCollector.countLogs` (elastic#67051) Remove unused license check result from LP Security plugin (elastic#66966) [Saved Objects] adds support for including hidden types in saved objects client (elastic#66879) [Discover] Deangularize timechart header (elastic#66532) [Discover] Improve and unskip a11y context view test (elastic#66959) [SIEM] Refactor Timeline.timelineType draft to Timeline.status draft (elastic#66864) ... # Conflicts: # x-pack/plugins/index_management/__jest__/client_integration/helpers/home.helpers.ts

* master: [apm] Annotation API documentation (elastic#65963) [Uptime] Improve responsiveness details page (elastic#67034) skip flaky suite (elastic#66669) Revert "Integration of a static filesystem for the node_modules (elastic#47998)" (elastic#67124) Support api_integration/kibana/stats against remote hosts (elastic#53000) chore(NA): add module name mapper for src plugins on x-pack (elastic#67103) Change the error message on TSVB in order to be more user friendly (elastic#67090) [kbn/optimizer] poll parent process to avoid zombie processes (elastic#67059) [Visualize] Lazy load default editor, fix duplicated styles (elastic#66732) Bump styled-component dependencies (elastic#66611) Bump react-markdown dependencies (elastic#66615) Fix Core docs links (elastic#66977) Timelion graph is not refreshing content after searching or filtering (elastic#67023) Remove `--xpack.endpoint.enabled=true` from README.md file (elastic#67053) Move apm tutorial from apm plugin into apm_oss plugin (elastic#66432) [Logs UI] Restore call to `UsageCollector.countLogs` (elastic#67051) Remove unused license check result from LP Security plugin (elastic#66966)

* master: (21 commits) [Alerting] Hides the `alert` SavedObjects type (elastic#66719) skip flaky suite (elastic#66869) fix visual baseline tests [kbn/optimizer] require fsevents on macos (elastic#67147) [APM] Fix obscured service map connections (elastic#67129) [apm] Annotation API documentation (elastic#65963) [Uptime] Improve responsiveness details page (elastic#67034) skip flaky suite (elastic#66669) Revert "Integration of a static filesystem for the node_modules (elastic#47998)" (elastic#67124) Support api_integration/kibana/stats against remote hosts (elastic#53000) chore(NA): add module name mapper for src plugins on x-pack (elastic#67103) Change the error message on TSVB in order to be more user friendly (elastic#67090) [kbn/optimizer] poll parent process to avoid zombie processes (elastic#67059) [Visualize] Lazy load default editor, fix duplicated styles (elastic#66732) Bump styled-component dependencies (elastic#66611) Bump react-markdown dependencies (elastic#66615) Fix Core docs links (elastic#66977) Timelion graph is not refreshing content after searching or filtering (elastic#67023) Remove `--xpack.endpoint.enabled=true` from README.md file (elastic#67053) Move apm tutorial from apm plugin into apm_oss plugin (elastic#66432) ...

sulemanof added 2 commits May 15, 2020 17:00

Lazy load bundle, fix duplicated styles

0cfda13

Add EuiLoadingChart and EuiErrorBoundary

7763e29

sulemanof requested review from flash1293 and timroes May 15, 2020 15:23

sulemanof changed the title ~~[Visualize] Lazy load bundle, fix duplicated styles~~ [Visualize] Lazy load default editor, fix duplicated styles May 15, 2020

sulemanof added 2 commits May 18, 2020 11:57

Return localStorage service

c28ff46

Merge remote-tracking branch 'upstream/master' into fix/editor_loading

68c9de1

flash1293 reviewed May 18, 2020

View reviewed changes

src/plugins/vis_default_editor/public/default_editor_controller.tsx Outdated Show resolved Hide resolved

sulemanof marked this pull request as ready for review May 18, 2020 12:43

sulemanof requested a review from a team May 18, 2020 12:43

sulemanof requested a review from a team as a code owner May 18, 2020 12:43

sulemanof added 3 commits May 19, 2020 14:17

Merge branch 'master' into fix/editor_loading

c5607a9

Use mono spinner

a998129

Get rid of loading the editor outside of visualize mounting

130dba3

sulemanof added release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v7.9.0 v8.0.0 Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels May 19, 2020

flash1293 approved these changes May 19, 2020

View reviewed changes

cchaos approved these changes May 20, 2020

View reviewed changes

sulemanof merged commit 66c24c5 into elastic:master May 20, 2020

sulemanof deleted the fix/editor_loading branch May 20, 2020 14:10

sulemanof mentioned this pull request May 20, 2020

[7.x] [Visualize] Lazy load default editor, fix duplicated styles (#66732) #67115

Merged

sulemanof mentioned this pull request May 20, 2020

[7.8] [Visualize] Lazy load default editor, fix duplicated styles (#66732) #67116

Merged

sulemanof added the backported label May 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Visualize] Lazy load default editor, fix duplicated styles#66732

[Visualize] Lazy load default editor, fix duplicated styles#66732
sulemanof merged 7 commits intoelastic:masterfrom
sulemanof:fix/editor_loading

sulemanof commented May 15, 2020 •

edited

Loading

Uh oh!

Uh oh!

flash1293 commented May 18, 2020

Uh oh!

mshustov commented May 18, 2020

Uh oh!

flash1293 commented May 18, 2020

Uh oh!

elasticmachine commented May 19, 2020

Uh oh!

flash1293 commented May 19, 2020

Uh oh!

flash1293 left a comment

Uh oh!

kibanamachine commented May 19, 2020

Uh oh!

cchaos left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

sulemanof commented May 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

Uh oh!

flash1293 commented May 18, 2020

Uh oh!

mshustov commented May 18, 2020

Uh oh!

flash1293 commented May 18, 2020

Uh oh!

elasticmachine commented May 19, 2020

Uh oh!

flash1293 commented May 19, 2020

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented May 19, 2020

💚 Build Succeeded

History

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sulemanof commented May 15, 2020 •

edited

Loading