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

[UnifiedFieldList] Convert from a plugin into a package #158718

Merged
merged 74 commits into from
Jun 23, 2023

Conversation

jughosta
Copy link
Contributor

@jughosta jughosta commented May 31, 2023

Summary

This PR converts unifiedFieldList plugin into a new @kbn/unified-field-list package.

Had to also move some deps:

  • from uiActions plugin to the existing @kbn/ui-actions-browser package
  • from data plugin to a new @kbn/data-service package

Please test that Field Stats from the package are still working on your pages.

@jughosta jughosta added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Feature:UnifiedFieldList The unified field list component used by Lens & Discover labels May 31, 2023
@jughosta jughosta self-assigned this May 31, 2023
@jughosta jughosta changed the title [UnifiedFieldList] Convert from a plugin to a package [UnifiedFieldList] Convert from a plugin into a package May 31, 2023
@ThomThomson ThomThomson added Feature:Embeddables Relating to the Embeddable system and removed Feature:Embedding Embedding content via iFrame labels Jun 14, 2023
@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Jun 14, 2023
Copy link
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.

Tested locally in Discover and Lens and everything seems to be working as intended, LGTM 👍 Thanks for the dependency untangling too!

On a side note, adding 250kb to the async bundles does seem like a lot (although saving 50kb on page load is good), and I'm not really a fan of having to reach into src folders to save on bundle size since it makes restructuring package internals later more difficult, but as @kertal mentioned these seem to be expected consequences of migrating to packages currently.

…gin-to-package

# Conflicts:
#	src/plugins/discover/tsconfig.json
@kertal
Copy link
Member

kertal commented Jun 15, 2023

On a side note, adding 250kb to the async bundles does seem like a lot (although saving 50kb on page load is good), and I'm not really a fan of having to reach into src folders to save on bundle size since it makes restructuring package internals later more difficult, but as @kertal mentioned these seem to be expected consequences of migrating to packages currently.

I do agree, there's definitely room for optimization beyond this PR, so something for optimizing the bundler where this part of the code can be a nice example for investigating it, FYI @elastic/kibana-operations
Also I assume breaking up the package in smaller packages (not in this PR of course) might reduce the size

@jughosta
Copy link
Contributor Author

Hi @elastic/kibana-design and @elastic/apm-ui,
Could you please review the PR as codeowners when you get a chance?

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.

Visualizations team changes LGTM, I also tested annotations and I don't see any regression. Not a fan of 300KB addition on the async bundle but I know that it can't be avoided 😓

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
aiops 525 566 +41
apm 1409 1457 +48
data 456 459 +3
dataVisualizer 344 348 +4
discover 465 559 +94
eventAnnotation 211 219 +8
lens 885 983 +98
ml 1743 1760 +17
unifiedFieldList 99 - -99
visualizationUiComponents 90 114 +24
total +238

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/data-service - 9 +9
@kbn/ui-actions-browser 8 35 +27
@kbn/unified-field-list - 267 +267
unifiedFieldList 270 - -270
total +33

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
@kbn/ui-actions-browser 0 2 +2

Async chunks

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

id before after diff
aiops 805.3KB 833.7KB +28.4KB
apm 3.6MB 3.6MB +36.2KB
data 52.8KB 52.8KB +4.0B
dataVisualizer 367.3KB 376.0KB +8.7KB
discover 423.0KB 513.7KB +90.7KB
eventAnnotation 116.7KB 120.6KB +3.9KB
lens 1.2MB 1.3MB +95.3KB
ml 3.4MB 3.4MB +24.6KB
unifiedFieldList 48.4KB - -48.4KB
visualizationUiComponents 26.3KB 38.7KB +12.3KB
total +251.6KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/unified-field-list - 5 +5
unifiedFieldList 8 - -8
total -3

Page load bundle

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

id before after diff
aiops 6.2KB 6.1KB -145.0B
apm 36.0KB 35.9KB -146.0B
data 406.4KB 406.5KB +55.0B
dataVisualizer 21.8KB 21.7KB -146.0B
discover 29.0KB 28.9KB -66.0B
eventAnnotation 27.7KB 27.6KB -90.0B
lens 35.4KB 35.3KB -88.0B
ml 73.1KB 72.9KB -146.0B
uiActions 20.1KB 20.1KB -4.0B
unifiedFieldList 53.1KB - -53.1KB
visualizationUiComponents 38.4KB 38.5KB +37.0B
total -53.8KB
Unknown metric groups

API count

id before after diff
@kbn/data-service - 14 +14
@kbn/ui-actions-browser 18 49 +31
@kbn/unified-field-list - 293 +293
unifiedFieldList 296 - -296
total +42

async chunk count

id before after diff
aiops 7 10 +3
apm 27 30 +3
discover 11 19 +8
eventAnnotation 4 5 +1
lens 14 21 +7
ml 31 34 +3
unifiedFieldList 7 - -7
visualizationUiComponents 1 2 +1
total +19

ESLint disabled line counts

id before after diff
@kbn/unified-field-list - 16 +16
enterpriseSearch 13 15 +2
securitySolution 416 420 +4
unifiedFieldList 18 - -18
total +4

References to deprecated APIs

id before after diff
@kbn/unified-field-list - 20 +20
unifiedFieldList 20 - -20
total -0

Total ESLint disabled count

id before after diff
@kbn/unified-field-list - 16 +16
enterpriseSearch 14 16 +2
securitySolution 497 501 +4
unifiedFieldList 18 - -18
total +4

History

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

cc @jughosta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Embeddables Relating to the Embeddable system Feature:Embedding Embedding content via iFrame Feature:UnifiedFieldList The unified field list component used by Lens & Discover release_note:skip Skip the PR/issue when compiling release notes Team:APM - DEPRECATED Use Team:obs-ux-infra_services. Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UnifiedFieldList] Convert UnifiedFieldList plugin into a package