Skip to content

remove sync register uiAction methods#254590

Merged
nreese merged 25 commits intoelastic:mainfrom
nreese:remove_ui_actions_sync
Apr 14, 2026
Merged

remove sync register uiAction methods#254590
nreese merged 25 commits intoelastic:mainfrom
nreese:remove_ui_actions_sync

Conversation

@nreese
Copy link
Copy Markdown
Contributor

@nreese nreese commented Feb 23, 2026

#205512 added 'async' registry methods. This PR completes this work and removes 'sync' registry methods and all remaining usages of 'sync ' methods.

async registries

  • avoid async imports during plugin setup/start
  • prevent race condition where plugins register items async. Another plugin's start contact could start before async setup is complete.

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 23, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 24, 2026

@elasticmachine merge upstream

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Feb 24, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 13, 2026

@elasticmachine merge upstream

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 13, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 13, 2026

/ci

Comment thread packages/kbn-optimizer/limits.yml Outdated
mapsEms: 6734
metricsDataAccess: 44950
ml: 89000
ml: 103469
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.

ML plugin async imports actions registration. This hides the true size of the ML plugin since part of it is hidden behind this async import.

This PR moves actions registration into ML plugin page load bundle size, and thus increases the bundle size to better reflect the actual size.

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 13, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 24, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 24, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 24, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 24, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 8, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 9, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Apr 9, 2026

/ci

@nreese nreese marked this pull request as ready for review April 9, 2026 20:10
@nreese nreese requested review from a team as code owners April 9, 2026 20:10
@nreese nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v9.5.0 labels Apr 9, 2026
@elasticmachine
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

Code review only, LGTM for the @elastic/security-threat-hunting-investigations team

mapsEms: 6734
metricsDataAccess: 44950
ml: 89000
ml: 102091
Copy link
Copy Markdown
Member

@tylersmalley tylersmalley Apr 9, 2026

Choose a reason for hiding this comment

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

Added a skill if you want to give it a try to reduce this. https://github.com/elastic/kibana/blob/main/.agents/skills/optimize-bundle-size/SKILL.md

/optimize-bundle-size pluginId=ml

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.

Code-only review, Data Discovery changes LGTM 👍


const { registerMlUiActions, registerSearchLinks, registerCasesAttachments } =
await import('./register_helper');
if (fullLicense && mlCapabilities.canGetMlInfo) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are the changes in this file necessary?
I can't see any changes in behaviour when reverting them.

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.

The changes in this file move registerMlUiActions from being imported async to being imported sync. This ensures that ui actions are registered sync instead of registered async.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But this is still happening after the license check and the call to getStartServices, so it is still async.

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.

thanks for pointing this out. I had not looked higher in the file and do see that all of these registrations are still inside an async function. At least this change removes one more await, with a network request, before registering actions - so its moving in the right direction. Completely fixing ML plugin registration is out of scope of this PR but it would be great if all registry items could be completed sync to avoid any timing issues.

registerMlUiActions(pluginsSetup.uiActions, core);
}

const { registerSearchLinks, registerCasesAttachments } = await import(
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 await import should be removed since loads an async file in plugin start, but that is out of scope for the PR.

Copy link
Copy Markdown
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

ML changes LGTM

@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
ml 4267 4268 +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 1.9MB 1.9MB +21.0B
ml 5.8MB 5.8MB +53.6KB
securitySolution 11.7MB 11.7MB +147.0B
total +53.8KB

Page load bundle

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

id before after diff
ml 86.8KB 90.7KB +3.9KB
uiActions 31.6KB 31.2KB -336.0B
total +3.6KB
Unknown metric groups

async chunk count

id before after diff
ml 112 113 +1

History

@nreese nreese merged commit 11ed364 into elastic:main Apr 14, 2026
19 checks passed
mbondyra added a commit to mbondyra/kibana that referenced this pull request Apr 14, 2026
* commit '11ed3645c5ededae2a6e29f2a79b31f52208b441': (157 commits)
  remove sync register uiAction methods (elastic#254590)
  [performance] Apply minimal auth to the search route (elastic#257497)
  [ES|QL] Reports correctly the controls server side errors (elastic#263020)
  [SecuritySolution][Navigation] Enable classic nav updates (elastic#262358)
  [Inference] Use pretty name and logo on feature settings page (elastic#262531)
  [Security Solution] fix AT-AB cypress test (elastic#262991)
  [SigEvents] Seed sigevents env script (elastic#261172)
  Adjust conditions for validating no refetch for expanded row (elastic#262978)
  [Agent Builder] update copy for the announcement modal (elastic#263034)
  [Search] Hide index management links for users without privileges (elastic#262627)
  Simplify OAS schema for GET `/api/spaces/space` query params (elastic#260831)
  Fix fleet output OAS regressions: SSL type explosion and Kafka union wrappers (elastic#260842)
  [Dashboards in chat] fix agent confusing the axes in a horizontal chat (elastic#263064)
  [One Workflow] Add alert state checkbox UI for workflow connector (elastic#259770)
  [One Workflow] Deprecate legacy Cases step types in workflow authoring (elastic#262070)
  skip failing test suite (elastic#248090)
  fix flaky test: MonitorDetails filter apply button not enabled (elastic#260788)
  fix: propagate AbortSignal to executeAsReasoningAgent for task cancellation (elastic#262811)
  [Security Solution][Alert KPI] Fix white space bug in alert KPIs (elastic#260803)
  [Streams] Move helpers and format_size_unit to utils folder (elastic#262550)
  ...

# Conflicts:
#	x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/canvas_integration/dashboard_canvas_content.test.tsx
#	x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/canvas_integration/dashboard_canvas_content.tsx
#	x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/canvas_integration/use_register_canvas_action_buttons.ts
#	x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/index.test.tsx
#	x-pack/platform/plugins/shared/dashboard_agent/public/attachment_types/index.tsx
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 release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants