Skip to content

Code-split kibanaReact & kibanaUtils#78140

Merged
Dosant merged 5 commits intoelastic:masterfrom
Dosant:dev/lazy-kibana-react
Sep 23, 2020
Merged

Code-split kibanaReact & kibanaUtils#78140
Dosant merged 5 commits intoelastic:masterfrom
Dosant:dev/lazy-kibana-react

Conversation

@Dosant
Copy link
Copy Markdown
Contributor

@Dosant Dosant commented Sep 22, 2020

Summary

This addresses most heavy things in kibanaReact & kibanaUtils:

Notes:

  • Turned out that enzyme is pretty ugly with suspense. After fighting it for sometime I re-implemented failing tests with testing-library
  • Snapshot update for autogenerated canvas stories. Not sure if there is way to handle it better.

@Dosant Dosant changed the title Code split kibanaReact & kibanaUtils CodeSplit kibanaReact & kibanaUtils Sep 22, 2020
@Dosant Dosant changed the title CodeSplit kibanaReact & kibanaUtils Code split kibanaReact & kibanaUtils Sep 22, 2020
Copy link
Copy Markdown
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.

Code LGTM.

By any chance do you have handy a screenshot of bundle analyzer output after this change?

@Dosant
Copy link
Copy Markdown
Contributor Author

Dosant commented Sep 22, 2020

@streamich , after the changes:

KibanaReact

Screenshot 2020-09-22 at 13 49 26

KibanaUtils

Screenshot 2020-09-22 at 13 49 00

@Dosant Dosant added Feature:kibana-react Team:AppArch technical debt Improvement of the software architecture and operational architecture v7.10.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Sep 22, 2020
@Dosant Dosant marked this pull request as ready for review September 22, 2020 18:35
@Dosant Dosant requested a review from a team September 22, 2020 18:35
@Dosant Dosant requested review from a team as code owners September 22, 2020 18:35
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@Dosant Dosant changed the title Code split kibanaReact & kibanaUtils Codesplit kibanaReact & kibanaUtils Sep 22, 2020
@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Sep 22, 2020
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I struggled with making this test work with enzyme for quite a while. Ended up with re-imlementing it with simpler @testing-library

@Dosant Dosant changed the title Codesplit kibanaReact & kibanaUtils Code-split kibanaReact & kibanaUtils Sep 22, 2020
@Dosant Dosant force-pushed the dev/lazy-kibana-react branch from 6630492 to 3255005 Compare September 23, 2020 10:55
onFocus={[Function]}
>
<div
className="react-monaco-editor-container"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤷‍♂️ this is autogenerated test. Didn't find a way how to wait for editor here.

Copy link
Copy Markdown
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

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

Only looked at Kibana App code. Code seems good to me

Copy link
Copy Markdown
Contributor

@crob611 crob611 left a comment

Choose a reason for hiding this comment

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

Code looks good to me but one little bit that needs to be cleaned up before merging though. See below 👇

@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
kibanaReact 388.6KB ⚠️ +388.6KB 0.0B
kibanaUtils 156.0KB +156.0KB 0.0B
total +544.6KB

page load bundle size

id value diff baseline
kibanaReact 266.7KB -379.3KB 646.0KB
kibanaUtils 318.9KB -152.8KB 471.7KB
total -532.1KB

distributable file count

id value diff baseline
default 45870 +21 45849
oss 26615 +21 26594
total +42

History

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

@Dosant Dosant merged commit e0f9323 into elastic:master Sep 23, 2020
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 Feature:kibana-react release_note:skip Skip the PR/issue when compiling release notes technical debt Improvement of the software architecture and operational architecture v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants