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

[embeddable] remove getAttributeService from start API #203660

Merged
merged 8 commits into from
Dec 12, 2024

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Dec 10, 2024

Part of embeddable refactor cleanup

AttributeService is moved from embeddable plugin to visualizations plugin.

PR reduces visualizations bundle size by avoiding importing legacy/embeddable/index.ts in plugin page load

@nreese
Copy link
Contributor Author

nreese commented Dec 10, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Dec 10, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Dec 10, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Dec 10, 2024

/ci

1 similar comment
@nreese
Copy link
Contributor Author

nreese commented Dec 10, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Dec 10, 2024

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Dec 10, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Dec 10, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Dec 10, 2024

/ci

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] x-pack/test_serverless/functional/test_suites/observability/common_configs/config.group5.ts / discover/group1 discover test nested query should support querying on nested fields

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
embeddable 142 140 -2
visualizations 493 494 +1
total -1

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
embeddable 458 436 -22

Async chunks

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

id before after diff
visualizations 320.4KB 328.9KB +8.4KB

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
embeddable 9 7 -2
visualizations 20 21 +1
total -1

Page load bundle

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

id before after diff
embeddable 71.0KB 68.8KB -2.2KB
visualizations 71.4KB 65.2KB -6.2KB
total -8.4KB
Unknown metric groups

API count

id before after diff
embeddable 564 541 -23

async chunk count

id before after diff
visualizations 15 16 +1

References to deprecated APIs

id before after diff
embeddable 18 16 -2
visualizations 58 60 +2
total -0

History

@nreese nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas release_note:skip Skip the PR/issue when compiling release notes v9.0.0 backport:version Backport to applied version labels v8.18.0 project:embeddableRebuild labels Dec 11, 2024
@nreese nreese marked this pull request as ready for review December 11, 2024 02:28
@nreese nreese requested review from a team as code owners December 11, 2024 02:28
@elasticmachine
Copy link
Contributor

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

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.

Code-only review, Data Discovery test change LGTM 👍

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Dec 11, 2024
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Changes LGTM! Code only review. The bundle size reduction is great!

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

i18n changes LGTM, reduces the bundles size too!

Copy link
Contributor

@dej611 dej611 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 👍

@nreese nreese merged commit 9ce5239 into elastic:main Dec 12, 2024
22 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12299747533

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 12, 2024
Part of embeddable refactor cleanup

AttributeService is moved from embeddable plugin to visualizations
plugin.

PR reduces visualizations bundle size by avoiding importing
`legacy/embeddable/index.ts` in plugin page load

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit 9ce5239)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Dec 12, 2024
#204059)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[embeddable] remove getAttributeService from start API
(#203660)](#203660)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Nathan
Reese","email":"reese.nathan@elastic.co"},"sourceCommit":{"committedDate":"2024-12-12T15:38:11Z","message":"[embeddable]
remove getAttributeService from start API (#203660)\n\nPart of
embeddable refactor cleanup\r\n\r\nAttributeService is moved from
embeddable plugin to visualizations\r\nplugin.\r\n\r\nPR reduces
visualizations bundle size by avoiding
importing\r\n`legacy/embeddable/index.ts` in plugin page
load\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"9ce523939279bdeba399b447849db47fa0877a23","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Embedding","Team:Presentation","release_note:skip","v9.0.0","project:embeddableRebuild","backport:version","v8.18.0"],"title":"[embeddable]
remove getAttributeService from start
API","number":203660,"url":"https://github.com/elastic/kibana/pull/203660","mergeCommit":{"message":"[embeddable]
remove getAttributeService from start API (#203660)\n\nPart of
embeddable refactor cleanup\r\n\r\nAttributeService is moved from
embeddable plugin to visualizations\r\nplugin.\r\n\r\nPR reduces
visualizations bundle size by avoiding
importing\r\n`legacy/embeddable/index.ts` in plugin page
load\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"9ce523939279bdeba399b447849db47fa0877a23"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203660","number":203660,"mergeCommit":{"message":"[embeddable]
remove getAttributeService from start API (#203660)\n\nPart of
embeddable refactor cleanup\r\n\r\nAttributeService is moved from
embeddable plugin to visualizations\r\nplugin.\r\n\r\nPR reduces
visualizations bundle size by avoiding
importing\r\n`legacy/embeddable/index.ts` in plugin page
load\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Elastic Machine
<elasticmachine@users.noreply.github.com>","sha":"9ce523939279bdeba399b447849db47fa0877a23"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Jan 13, 2025
Part of embeddable refactor cleanup

AttributeService is moved from embeddable plugin to visualizations
plugin.

PR reduces visualizations bundle size by avoiding importing
`legacy/embeddable/index.ts` in plugin page load

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels Feature:Embedding Embedding content via iFrame project:embeddableRebuild release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants