Skip to content

[Embeddable Rebuild] [Discover] Fix bundle size#189206

Merged
Heenawter merged 6 commits intoelastic:mainfrom
Heenawter:embeddableRebuild_fixDiscoverBundleSize_2024-07-25
Jul 30, 2024
Merged

[Embeddable Rebuild] [Discover] Fix bundle size#189206
Heenawter merged 6 commits intoelastic:mainfrom
Heenawter:embeddableRebuild_fixDiscoverBundleSize_2024-07-25

Conversation

@Heenawter
Copy link
Copy Markdown
Contributor

@Heenawter Heenawter commented Jul 25, 2024

Summary

This PR is a follow up to #180536, where 8.8KB was added to the Discover bundle size - now, after async importing from the presentation-publishing package where necessary, I've reduced that increase to 3.4KB (a decrease of 5.4KB from the previous PR).

For maintainers

@Heenawter Heenawter added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels Jul 25, 2024
@Heenawter Heenawter self-assigned this Jul 25, 2024
@Heenawter Heenawter force-pushed the embeddableRebuild_fixDiscoverBundleSize_2024-07-25 branch from a1c0bac to d107fac Compare July 25, 2024 16:40
@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter Heenawter marked this pull request as ready for review July 25, 2024 21:54
@Heenawter Heenawter requested a review from a team as a code owner July 25, 2024 21:54
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

Copy link
Copy Markdown
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this! Just left one question/suggestion I think could be worth trying before approving.

Comment on lines +32 to +34
const { apiCanAccessViewMode, apiHasType, apiIsOfType, getInheritedViewMode } = await import(
'@kbn/presentation-publishing'
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One downside to this approach is that I think our async bundle will now grow with @kbn/presentation-publishing since we're importing the entire package here. I wonder if it would work to keep the imports how they were for tree shaking, but break getCompatibilityCheck out into a separate file and async import that here instead?

Copy link
Copy Markdown
Contributor Author

@Heenawter Heenawter Jul 29, 2024

Choose a reason for hiding this comment

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

Oh!! Genius 🤯 Good call on that one - did this in 5080816 and it reduced the async bundle size increase of this PR by about 50% (I think it was around +12KB before, and it's +6KB now).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome 🤘 thank you!

Copy link
Copy Markdown
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for addressing this!

@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
discover 957 958 +1

Async chunks

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

id before after diff
discover 816.8KB 822.8KB +6.0KB

Page load bundle

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

id before after diff
discover 52.7KB 47.3KB -5.4KB
Unknown metric groups

async chunk count

id before after diff
discover 25 27 +2

History

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

cc @Heenawter

@Heenawter Heenawter merged commit a35f773 into elastic:main Jul 30, 2024
@Heenawter Heenawter deleted the embeddableRebuild_fixDiscoverBundleSize_2024-07-25 branch July 30, 2024 15:46
@kibanamachine kibanamachine added v8.16.0 backport:skip This PR does not require backporting labels Jul 30, 2024
nreese added a commit that referenced this pull request Jul 30, 2024
…189533)

#189128 increases
@kbn/presentation-containers size. This causes synthetics page load
bundle to exceed the bundle size limit. Synthetics page load should not
include @kbn/presentation-containers. This PR resolves the problem by
putting @kbn/presentation-containers load behind an async import

Note for reviewers, put compatibility check into its own file to avoid
importing entire @kbn/presentation-containers
into async bundle. Tree shaking is enable by putting the import in a
seperate file - see
#189206 (comment) for
more details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort project:embeddableRebuild release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants