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

Make the actual Vislib import async #78949

Merged
merged 3 commits into from
Oct 1, 2020
Merged

Conversation

timroes
Copy link
Contributor

@timroes timroes commented Sep 30, 2020

Summary

Import Vislib async.

This PR optimizes the page load bundle size of the vis_type_vislib plugin by making sure that the Vislib class which is eventually responsible for drawing is imported async. This is easily possible, since it was anyway already instantiated inside an async method.

This PR should not change any functionality.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@timroes timroes added Feature:Vislib Vislib chart implementation performance Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Sep 30, 2020
@timroes timroes requested review from stratoula, sulemanof and a team September 30, 2020 13:22
@elasticmachine
Copy link
Contributor

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

@timroes
Copy link
Contributor Author

timroes commented Sep 30, 2020

@elasticmachine merge upstream

Copy link
Contributor

@sulemanof sulemanof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worthy improvement! ❤️

@timroes
Copy link
Contributor Author

timroes commented Oct 1, 2020

@elasticmachine merge upstream

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Daniil 😄 LGTM!

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in Chrome and Safari, vislib charts still render fine on dashboards, editor and canvas

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
visTypeVislib 0.0B 593.2KB ⚠️ +593.2KB

distributable file count

id before after diff
default 45831 45837 +6
oss 26554 26560 +6

page load bundle size

id before after diff
visTypeVislib 812.2KB 222.8KB -589.3KB

History

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

@timroes timroes merged commit 9b187c5 into elastic:master Oct 1, 2020
@timroes timroes deleted the optimize/vislib branch October 1, 2020 09:10
timroes pushed a commit to timroes/kibana that referenced this pull request Oct 1, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Oct 1, 2020
* master: (36 commits)
  [I18n] fix eui tokens (elastic#78951)
  Changed the color of the confirm button in trusted app deletion dialog. (elastic#78768)
  Make the actual Vislib import async (elastic#78949)
  Fix ML conditionals links Cypress tests (elastic#78568)
  [Drilldowns][Docs] Communicate the visualization types that support drilldowns (elastic#78761)
  [UX] Improve page-load axis (elastic#78392)
  [SECURITY SOLUTIONS] Map embeddable working with index patterns selection (elastic#78610)
  Data plugin README (elastic#78750)
  [TSVB] Request validation error: [panels.0.series.0.metrics.0.percentiles.1.value] (elastic#79009)
  fixing api test (elastic#78964)
  [Task names in TaskManager] Rename "telemetry" to "usage" (elastic#78129)
  [Loggers] Rename "telemetry" to "usage" (elastic#78130)
  [Usage Collection] [schema] `ui_metric` (elastic#78827)
  [Actions][Jira] Set parent issue for Sub-task issue type (elastic#78772)
  [Discover] Unskip doc link functional test (elastic#78600)
  [ML] Functional tests - stabilize calendar edit tests (elastic#78950)
  [UX] Improve page responsive  (elastic#78759)
  [QA][Code Coverage] Team Assignment Docs Update (elastic#78890)
  [ML] Migrate machine learning URLs to BrowserRouter format for APM, Security, and Infra  (elastic#78209)
  [ts] enable "resolveJsonModule" and disable existing failures (elastic#78855)
  ...
timroes pushed a commit that referenced this pull request Oct 1, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@timroes timroes added the hacktoberfest-accepted https://hacktoberfest.digitalocean.com/hacktoberfest-update label Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Vislib Vislib chart implementation hacktoberfest-accepted https://hacktoberfest.digitalocean.com/hacktoberfest-update performance release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants