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

[Lens] Improve Lens initial bundle size #79292

Merged
merged 1 commit into from Oct 2, 2020

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Oct 2, 2020

This PR reduces the initial bundle size of Lens by moving a few more files behind the lazy import boundary in async_services.

Most of the changes are due to the fact the embeddable got the static toExpression function of the interpreter passed in as parameter (I guess this was done to simplify unit testing). I switched to a different approach here (mocking the result of documentToExpression) which made that unnecessary.

@flash1293 flash1293 added Feature:Lens release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 labels Oct 2, 2020
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
lens 561 562 +1

async chunks size

id before after diff
lens 1010.8KB 1.1MB +65.7KB

page load bundle size

id before after diff
lens 139.9KB 76.0KB -63.8KB

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

@flash1293 flash1293 marked this pull request as ready for review October 2, 2020 15:55
@flash1293 flash1293 requested a review from a team October 2, 2020 15:55
@flash1293 flash1293 added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Oct 2, 2020
@elasticmachine
Copy link
Contributor

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

@@ -83,6 +83,8 @@ export class EmbeddableFactory implements EmbeddableFactoryDefinition {
indexPatternService,
} = await this.getStartServices();

const { Embeddable } = await import('../../async_services');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

loading the actual embeddable async when required

@@ -7,7 +7,7 @@
import { Capabilities, HttpSetup } from 'kibana/public';
import { i18n } from '@kbn/i18n';
import { RecursiveReadonly } from '@kbn/utility-types';
import { toExpression, Ast } from '@kbn/interpreter/target/common';
import { Ast } from '@kbn/interpreter/target/common';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't load code from interpreter within the factory (because it's part of the initial bundle)

@@ -25,10 +25,8 @@ import { Document } from '../persistence/saved_object_store';
import { mergeTables } from './merge_tables';
import { formatColumn } from './format_column';
import { EmbeddableFactory, LensEmbeddableStartServices } from './embeddable/embeddable_factory';
import { getActiveDatasourceIdFromDoc } from './editor_frame/state_management';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Load some helpers async when needed because they pull in a tail of other modules

Copy link
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

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

LGTM, tested in firefox

@flash1293 flash1293 merged commit b66de2c into elastic:master Oct 2, 2020
flash1293 added a commit to flash1293/kibana that referenced this pull request Oct 5, 2020
flash1293 added a commit that referenced this pull request Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens 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.

None yet

4 participants