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

[kbn/optimizer] link to kibanaReact/kibanaUtils plugins #62720

Merged
merged 30 commits into from
Apr 8, 2020

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Apr 6, 2020

In order to deal with the excessive size of Kibana platform plugin bundles, caused by a number of issues (outlined in #62238), this PR updates the bootstrap logic to load the kibanaReact, and kibanaUtils plugins before the application/core bundle and rewrites all imports to plugins/{id}/public for these two plugins to reference the exports from the plugin bundles directly, without bundling their source directly into the separate plugin bundles.

We can merge this into master and backport to 7.7 to keep things consistent, but there are a few plans bouncing around that might make this unnecessary as soon as 7.8.

@spalger
Copy link
Contributor Author

spalger commented Apr 7, 2020

@elasticmachine merge upstream

@spalger spalger added v7.7.0 v7.8.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team labels Apr 7, 2020
@elasticmachine
Copy link
Contributor

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

@spalger spalger marked this pull request as ready for review April 7, 2020 00:13
@spalger spalger requested review from a team as code owners April 7, 2020 00:13
@tylersmalley
Copy link
Contributor

tylersmalley commented Apr 7, 2020

I am getting Definition of plugin "kibanaUtils" should be a function (/bundles/plugin/kibanaUtils/kibanaUtils.plugin.js). in IE.

@mshustov

This comment has been minimized.

@mshustov
Copy link
Contributor

mshustov commented Apr 7, 2020

@spalger there is https://github.com/elastic/kibana/tree/f86dac77da8bcd16d12d41f27bc34220a2ac8b82/src/plugins/es_ui_shared used by several plugins. Shouldn't we add it to the current PR as it contains quite a heavy brace editor?

@spalger
Copy link
Contributor Author

spalger commented Apr 7, 2020

@elasticmachine merge upstream

@spalger spalger changed the title [kbn/optimizer] link to data/kibanaReact/kibanaUtils plugins [kbn/optimizer] link to kibanaReact/kibanaUtils plugins Apr 8, 2020
Copy link
Contributor

@tylersmalley tylersmalley left a comment

Choose a reason for hiding this comment

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

LGTM. This reduces the downloaded JS assets by 5MB. Setting async=false in the bootstrap loader results in more efficient parallelizing of download while still maintaining the execution order.

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.

Kibana app changes LGTM, just changing imports to public instead of common. Code review only

@flash1293
Copy link
Contributor

flash1293 commented Apr 8, 2020

@spalger We are currently working on moving the vis_default_editor plugin and all of the individual visualizations (markdown, vislib, vega, tag cloud, ...) to the new platform. They will probably have a similar problem as kibana_utils and kibana_react, because vis_default_editor is just a static code bundle imported by a lot of other plugins. It seems possible to use the same trick there.

but there are a few plans bouncing around that might make this unnecessary as soon as 7.8

This would be even better, just mentioning it here in case it becomes relevant.

…k-data-plugin

# Conflicts:
#	src/legacy/core_plugins/kibana/public/discover/kibana_services.ts
@spalger
Copy link
Contributor Author

spalger commented Apr 8, 2020

Skipped flaky test #59030

Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

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

AppArch changes LGTM.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@spalger spalger merged commit 369ddff into elastic:master Apr 8, 2020
spalger added a commit to spalger/kibana that referenced this pull request Apr 8, 2020
* [kbn/optimizer] link to data/kibanaReact/kibanaUtils plugins

* depend on normalize-path package

* typos

* avoid loading kibanaUtils and kibanaReact from urls

* update types and tests, now that whole plugin is exported to window

* update snapshot, removed export of `plugins` property

* fix condition, ignore things NOT in data/react/utils

* make es_ui_shared a "static bundle" too

* move kibana_utils/common usage to /public

* convert some more /common usage to /public

* use async-download/ordered-execution for bootstrap script

* fix typo

* remove kibanaUtils bundle

* remove kibanaReact bundle

* Revert "remove kibanaReact bundle"

This reverts commit f14e9ee.

* Revert "remove kibanaUtils bundle"

This reverts commit a64b2a7.

* stop linking to the data plugin

* add comment pointing to async-download info

Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
spalger pushed a commit to spalger/kibana that referenced this pull request Apr 8, 2020
* [kbn/optimizer] link to data/kibanaReact/kibanaUtils plugins

* depend on normalize-path package

* typos

* avoid loading kibanaUtils and kibanaReact from urls

* update types and tests, now that whole plugin is exported to window

* update snapshot, removed export of `plugins` property

* fix condition, ignore things NOT in data/react/utils

* make es_ui_shared a "static bundle" too

* move kibana_utils/common usage to /public

* convert some more /common usage to /public

* use async-download/ordered-execution for bootstrap script

* fix typo

* remove kibanaUtils bundle

* remove kibanaReact bundle

* Revert "remove kibanaReact bundle"

This reverts commit f14e9ee.

* Revert "remove kibanaUtils bundle"

This reverts commit a64b2a7.

* stop linking to the data plugin

* add comment pointing to async-download info

Co-authored-by: spalger <spalger@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
# Conflicts:
#	src/legacy/core_plugins/kibana/public/discover/kibana_services.ts
#	src/legacy/core_plugins/vis_type_metric/public/services.ts
#	src/legacy/core_plugins/vis_type_table/public/services.ts
#	src/legacy/core_plugins/vis_type_vislib/public/services.ts
#	src/legacy/core_plugins/visualizations/public/np_ready/public/legacy.ts
@spalger spalger deleted the implement/link-data-plugin branch April 8, 2020 21:28
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 9, 2020
…chore/put-all-xjson-together

* 'master' of github.com:elastic/kibana: (35 commits)
  [SIEM] [Detection Engine] Fixes bug when notification doesn't… (elastic#63013)
  [SIEM][Detection Engine] Fix rule notification critical bugs
  Add Error Exception Type Column (elastic#59596)
  [APM] Agent remote configuration: changes in Java property descriptions (elastic#62282)
  [Alerting] Displays warning when a permanent encryption key is missing and hides alerting UI appropriately (elastic#62772)
  FTR: add chromium-based Edge browser support (elastic#61684)
  [Ingest] Data source configuration validation UI (elastic#61180)
  restore empty_kibana after saved objects test (elastic#62951)
  Index pattern management plugin - src/legacy/core_plugins/management => new platform plugin (elastic#62594)
  Add basic StatusService (elastic#60335)
  [kbn/optimizer] link to kibanaReact/kibanaUtils plugins (elastic#62720)
  [APM] Service map - fixes layout issues for maps with no rum services (elastic#62887)
  Exclude disabled datasources and streams from agent config (elastic#62869)
  [Alerting] Fix validation support for nested IErrorObjects (elastic#62833)
  [Metrics UI] Invalidate non-count alerts which have no metrics (elastic#62837)
  Add --filter option to API docs script (elastic#62888)
  [Maps] fix attribution overflow with exit full screen button (elastic#62699)
  [Uptime]Alerting UI text in case filter is selected (elastic#62570)
  [Maps] Show create filter button for top-term tooltip property (elastic#62461)
  skip flaky suite (elastic#59030)
  ...

# Conflicts:
#	src/plugins/es_ui_shared/public/index.ts
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 9, 2020
* master: (40 commits)
  [ML] Functional transform tests - stabilize source selection (elastic#63087)
  add embed flag to saved object url as well (elastic#62926)
  [SIEM] [Detection Engine] Fixes bug when notification doesn't… (elastic#63013)
  [SIEM][Detection Engine] Fix rule notification critical bugs
  Add Error Exception Type Column (elastic#59596)
  [APM] Agent remote configuration: changes in Java property descriptions (elastic#62282)
  [Alerting] Displays warning when a permanent encryption key is missing and hides alerting UI appropriately (elastic#62772)
  FTR: add chromium-based Edge browser support (elastic#61684)
  [Ingest] Data source configuration validation UI (elastic#61180)
  restore empty_kibana after saved objects test (elastic#62951)
  Index pattern management plugin - src/legacy/core_plugins/management => new platform plugin (elastic#62594)
  Add basic StatusService (elastic#60335)
  [kbn/optimizer] link to kibanaReact/kibanaUtils plugins (elastic#62720)
  [APM] Service map - fixes layout issues for maps with no rum services (elastic#62887)
  Exclude disabled datasources and streams from agent config (elastic#62869)
  [Alerting] Fix validation support for nested IErrorObjects (elastic#62833)
  [Metrics UI] Invalidate non-count alerts which have no metrics (elastic#62837)
  Add --filter option to API docs script (elastic#62888)
  [Maps] fix attribution overflow with exit full screen button (elastic#62699)
  [Uptime]Alerting UI text in case filter is selected (elastic#62570)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.7.0 v7.8.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants