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

[bundle optimization] fix imports of react-use lib #82847

Merged
merged 8 commits into from
Nov 10, 2020

Conversation

alexwizp
Copy link
Contributor

@alexwizp alexwizp commented Nov 6, 2020

Summary

Currently, when importing from the react-use library, we are importing all the code, which increases the size of our final bundle. A simple fix for the import solves this problem

in total -1.3 MB 🥳

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@alexwizp alexwizp self-assigned this Nov 6, 2020
@alexwizp alexwizp added Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0 Feature:Build Packaging release_note:skip Skip the PR/issue when compiling release notes labels Nov 6, 2020
@alexwizp alexwizp requested review from stratoula and timroes November 6, 2020 15:36
@alexwizp alexwizp marked this pull request as ready for review November 6, 2020 17:16
@alexwizp alexwizp requested review from a team as code owners November 6, 2020 17:16
@alexwizp alexwizp requested review from a team November 6, 2020 17:16
@alexwizp alexwizp requested review from a team as code owners November 6, 2020 17:16
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@jfsiii jfsiii left a comment

Choose a reason for hiding this comment

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

LGTM for Ingest Manager. Thanks! We documented the issue and fix as part of #82160

@botelastic botelastic bot added Team:APM All issues that need APM UI Team support Team:Fleet Team label for Observability Data Collection Fleet team Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Nov 6, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

Copy link
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

LGTM

@alexwizp
Copy link
Contributor Author

alexwizp commented Nov 8, 2020

@elasticmachine merge upstream

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

Transform changes LGTM

@afgomez afgomez self-requested a review November 9, 2020 09:59
Copy link
Contributor

@afgomez afgomez left a comment

Choose a reason for hiding this comment

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

Infra changes LGTM.

I wonder if we have explored automating this via babel-plugin-transform-imports. I don't know anything about our build system though, so maybe it's not possible.

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.

Kibana App changes LGTM!

Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

apm changes lgtm

@alexwizp
Copy link
Contributor Author

alexwizp commented Nov 9, 2020

@elasticmachine merge upstream

@alexwizp
Copy link
Contributor Author

alexwizp commented Nov 9, 2020

ping @elastis/siem @elastic/uptime @elastic/endpoint-app-team

@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@alexwizp
Copy link
Contributor Author

@elasticmachine merge upstream

@patrykkopycinski
Copy link
Contributor

@alexwizp can we add some check to make sure we don't break this pattern in the future again? Maybe babel-plugin to rewrite those paths automatically or at least eslint rule like here?
https://github.com/elastic/kibana/blob/master/.eslintrc.js#L597-L604

Copy link
Contributor

@andrewvc andrewvc left a comment

Choose a reason for hiding this comment

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

LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
apm 1275 1157 -118
home 207 79 -128
infra 1145 1036 -109
ingestManager 535 408 -127
lens 559 431 -128
logstash 189 61 -128
transform 337 209 -128
uptime 657 529 -128
visDefaultEditor 240 115 -125
visualize 205 78 -127
total -1246

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.2MB 3.1MB -127.8KB
home 348.4KB 214.4KB -134.0KB
infra 2.6MB 2.5MB -87.9KB
ingestManager 1.2MB 1.1MB -133.6KB
lens 1.0MB 909.3KB -133.3KB
logstash 194.4KB 60.8KB -133.6KB
transform 1.1MB 1.0MB -133.9KB
uptime 1.1MB 983.1KB -134.0KB
visDefaultEditor 431.6KB 299.4KB -132.1KB
visualize 259.7KB 126.2KB -133.5KB
total -1.3MB

Distributable file count

id before after diff
default 42766 42763 -3

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
home 20.9KB 20.9KB -1.0B
infra 174.2KB 174.0KB -151.0B
logstash 25.9KB 25.9KB -1.0B
transform 24.5KB 24.5KB -1.0B
uptime 25.2KB 25.1KB -65.0B
visualize 40.1KB 40.1KB -1.0B
total -220.0B
Unknown metric groups

async chunk count

id before after diff
infra 15 14 -1

History

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

Copy link
Contributor

@patrykkopycinski patrykkopycinski left a comment

Choose a reason for hiding this comment

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

SIEM/Endpoint LGTM

@alexwizp alexwizp merged commit b2d6b66 into elastic:master Nov 10, 2020
alexwizp added a commit to alexwizp/kibana that referenced this pull request Nov 10, 2020
* [bundle optimization] fix imports of react-use lib

* add 2 more files

* add rule into eslintrc.js

Co-authored-by: Kibana Machine <[email protected]>
alexwizp added a commit that referenced this pull request Nov 10, 2020
* [bundle optimization] fix imports of react-use lib

* add 2 more files

* add rule into eslintrc.js

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
phillipb added a commit to phillipb/kibana that referenced this pull request Nov 10, 2020
…kibana into bootstrap-node-details-overlay

* 'bootstrap-node-details-overlay' of github.com:phillipb/kibana: (49 commits)
  [Security Solution] Fix DNS Network table query (elastic#82778)
  [Workplace Search] Consolidate groups routes (elastic#83015)
  Adds cloud links to user menu (elastic#82803)
  [Security Solution][Detections] - follow up cleanup on auto refresh rules (elastic#83023)
  [App Search] Added the log retention panel to the Settings page (elastic#82982)
  [Maps] show icon when layer is filtered by time and allow layers to ignore global time range (elastic#83006)
  [DOCS] Consolidates drilldown pages (elastic#82081)
  [Maps] add on-prem EMS config (elastic#82525)
  migrate i18n mixin to KP (elastic#81799)
  [bundle optimization] fix imports of react-use lib (elastic#82847)
  [Discover] Add metric on adding filter (elastic#82961)
  [Lens] Performance refactoring for indexpattern fast lookup and Operation support matrix computation (elastic#82829)
  skip flaky suite (elastic#82804)
  Fix SO query for searching across spaces (elastic#83025)
  renaming built-in alerts to Stack Alerts (elastic#82873)
  [TSVB] Disable using top_hits in pipeline aggregations (elastic#82278)
  [Visualizations] Remove kui usage (elastic#82810)
  [Visualizations] Make the icon buttons labels more descriptive (elastic#82585)
  [Lens] Do not reset formatting when switching between custom ranges and auto histogram (elastic#82694)
  Fix ilm navigation (elastic#81664)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 10, 2020
…na into alerts/stack-alerts-public

* 'alerts/stack-alerts-public' of github.com:gmmorris/kibana:
  [Security Solution] Fix DNS Network table query (elastic#82778)
  [Workplace Search] Consolidate groups routes (elastic#83015)
  Adds cloud links to user menu (elastic#82803)
  [Security Solution][Detections] - follow up cleanup on auto refresh rules (elastic#83023)
  [App Search] Added the log retention panel to the Settings page (elastic#82982)
  [Maps] show icon when layer is filtered by time and allow layers to ignore global time range (elastic#83006)
  [DOCS] Consolidates drilldown pages (elastic#82081)
  [Maps] add on-prem EMS config (elastic#82525)
  migrate i18n mixin to KP (elastic#81799)
  [bundle optimization] fix imports of react-use lib (elastic#82847)
  [Discover] Add metric on adding filter (elastic#82961)
  [Lens] Performance refactoring for indexpattern fast lookup and Operation support matrix computation (elastic#82829)
  skip flaky suite (elastic#82804)
  Fix SO query for searching across spaces (elastic#83025)
  renaming built-in alerts to Stack Alerts (elastic#82873)
  [TSVB] Disable using top_hits in pipeline aggregations (elastic#82278)
  [Visualizations] Remove kui usage (elastic#82810)
  [Visualizations] Make the icon buttons labels more descriptive (elastic#82585)
  [Lens] Do not reset formatting when switching between custom ranges and auto histogram (elastic#82694)
:
@alexwizp alexwizp deleted the react-use branch January 16, 2021 15:27
@MindyRS MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 22, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Build Packaging release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:Fleet Team label for Observability Data Collection Fleet team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.