[uiActions] make trigger action registry async#205512
Conversation
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
davismcphee
left a comment
There was a problem hiding this comment.
Checked quickly in Discover and no issues, Data Discovery changes LGTM 👍
| const results = await Promise.allSettled([ | ||
| startServicesPromise, | ||
| componentPromise, | ||
| modulePromise, | ||
| import('./panel_module'), | ||
| ]); | ||
| const Panel = panelModule.PresentationPanelInternal; | ||
| return { Panel, unwrappedComponent }; | ||
|
|
||
| for (const result of results) { | ||
| if (result.status === 'rejected') { | ||
| throw new Error(result.reason); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: all this code looks very similar to the default behaviour of Promise.all? Why checking it manually?
There was a problem hiding this comment.
Promise.all does not return values when any promise rejects. Since PresentationPanelErrorInternal is returned from one of the promises (and PresentationPanelErrorInternal is used to diplay errors), I am using Promise. allSettled so PresentationPanelErrorInternal is returned if componentPromise rejects.
There was a problem hiding this comment.
I do see a logic error now where the method throws when a promise rejects so no values are returned. I will rework this so that it does not throw on rejected, and instead returns an error status along with any available results
|
@elasticmachine merge upstream |
eokoneyo
left a comment
There was a problem hiding this comment.
Shared UX changes LGTM, thanks for the contribution
ThomThomson
left a comment
There was a problem hiding this comment.
This is such a straightforward set of changes for such an impactful reduction in bundle size! Excellent idea and execution here.
Looked through the code and everything LGTM!
Also ran this locally and verified that the presentationPanel plugin had only one async chunk, and that it was loaded when expected (i.e. when a Dashboard with at least one panel is on screen).
|
|
||
| useEffect(() => { | ||
| let cancelled = false; | ||
| let canceled = false; |
There was a problem hiding this comment.
Canadian vs American spelling here 😈 CC @Heenawter for backup
walterra
left a comment
There was a problem hiding this comment.
ML related changes LGTM. This is a nice enhancement, we're looking forward to make use of it.
|
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
References to deprecated APIs
Total ESLint disabled count
History
|
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/12752165266 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Closes elastic#191642, part of elastic#205512 PR makes the following changes to uiActions API. * Deprecates `registerAction` and `addTriggerAction` and replaces them with `registerActionAsync` and `addTriggerActionAsync` * Makes all registry consumption methods async, such as `getAction`, `getTriggerActions` and `getFrequentlyChangingActionsForTrigger` PR updates presentation_panel plugin to use `registerActionAsync`. With actions behind async import, page load bundle size has been reduced by 21.1KB. <img width="500" alt="Screenshot 2025-01-08 at 2 14 23 PM" src="https://github.com/user-attachments/assets/34a2cae9-dc5e-429b-bbdb-ffd9dfe1cce3" /> Also, async exports are [contained in a single file `panel_module`](elastic#206117). This results in dashboard only loading one `presentationPanel.chunk`. <img width="500" alt="Screenshot 2025-01-08 at 2 15 02 PM" src="https://github.com/user-attachments/assets/e083b852-b50d-4fa7-8ebd-e2f56f85e998" /> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 1bd8c97) # Conflicts: # src/platform/plugins/private/presentation_panel/public/panel_component/presentation_panel.tsx
# Backport This will backport the following commits from `main` to `8.x`: - [[uiActions] make trigger action registry async (#205512)](#205512) <!--- Backport version: 8.9.8 --> ### 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":"2025-01-13T16:57:34Z","message":"[uiActions] make trigger action registry async (#205512)\n\nCloses #191642, part of\r\nhttps://github.com//pull/205512\r\n\r\nPR makes the following changes to uiActions API.\r\n* Deprecates `registerAction` and `addTriggerAction` and replaces them\r\nwith `registerActionAsync` and `addTriggerActionAsync`\r\n* Makes all registry consumption methods async, such as `getAction`,\r\n`getTriggerActions` and `getFrequentlyChangingActionsForTrigger`\r\n\r\nPR updates presentation_panel plugin to use `registerActionAsync`. With\r\nactions behind async import, page load bundle size has been reduced by\r\n21.1KB.\r\n\r\n<img width=\"500\" alt=\"Screenshot 2025-01-08 at 2 14 23 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/34a2cae9-dc5e-429b-bbdb-ffd9dfe1cce3\"\r\n/>\r\n\r\nAlso, async exports are [contained in a single file\r\n`panel_module`](#206117). This\r\nresults in dashboard only loading one `presentationPanel.chunk`.\r\n\r\n<img width=\"500\" alt=\"Screenshot 2025-01-08 at 2 15 02 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/e083b852-b50d-4fa7-8ebd-e2f56f85e998\"\r\n/>\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":"1bd8c97982146a4643d6eca1e21b525a5c8d456f","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","release_note:skip","v9.0.0","backport:version","v8.18.0"],"number":205512,"url":"https://github.com/elastic/kibana/pull/205512","mergeCommit":{"message":"[uiActions] make trigger action registry async (#205512)\n\nCloses #191642, part of\r\nhttps://github.com//pull/205512\r\n\r\nPR makes the following changes to uiActions API.\r\n* Deprecates `registerAction` and `addTriggerAction` and replaces them\r\nwith `registerActionAsync` and `addTriggerActionAsync`\r\n* Makes all registry consumption methods async, such as `getAction`,\r\n`getTriggerActions` and `getFrequentlyChangingActionsForTrigger`\r\n\r\nPR updates presentation_panel plugin to use `registerActionAsync`. With\r\nactions behind async import, page load bundle size has been reduced by\r\n21.1KB.\r\n\r\n<img width=\"500\" alt=\"Screenshot 2025-01-08 at 2 14 23 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/34a2cae9-dc5e-429b-bbdb-ffd9dfe1cce3\"\r\n/>\r\n\r\nAlso, async exports are [contained in a single file\r\n`panel_module`](#206117). This\r\nresults in dashboard only loading one `presentationPanel.chunk`.\r\n\r\n<img width=\"500\" alt=\"Screenshot 2025-01-08 at 2 15 02 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/e083b852-b50d-4fa7-8ebd-e2f56f85e998\"\r\n/>\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":"1bd8c97982146a4643d6eca1e21b525a5c8d456f"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205512","number":205512,"mergeCommit":{"message":"[uiActions] make trigger action registry async (#205512)\n\nCloses #191642, part of\r\nhttps://github.com//pull/205512\r\n\r\nPR makes the following changes to uiActions API.\r\n* Deprecates `registerAction` and `addTriggerAction` and replaces them\r\nwith `registerActionAsync` and `addTriggerActionAsync`\r\n* Makes all registry consumption methods async, such as `getAction`,\r\n`getTriggerActions` and `getFrequentlyChangingActionsForTrigger`\r\n\r\nPR updates presentation_panel plugin to use `registerActionAsync`. With\r\nactions behind async import, page load bundle size has been reduced by\r\n21.1KB.\r\n\r\n<img width=\"500\" alt=\"Screenshot 2025-01-08 at 2 14 23 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/34a2cae9-dc5e-429b-bbdb-ffd9dfe1cce3\"\r\n/>\r\n\r\nAlso, async exports are [contained in a single file\r\n`panel_module`](#206117). This\r\nresults in dashboard only loading one `presentationPanel.chunk`.\r\n\r\n<img width=\"500\" alt=\"Screenshot 2025-01-08 at 2 15 02 PM\"\r\nsrc=\"https://github.com/user-attachments/assets/e083b852-b50d-4fa7-8ebd-e2f56f85e998\"\r\n/>\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":"1bd8c97982146a4643d6eca1e21b525a5c8d456f"}},{"branch":"8.x","label":"v8.18.0","labelRegex":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
Closes elastic#191642, part of elastic#205512 PR makes the following changes to uiActions API. * Deprecates `registerAction` and `addTriggerAction` and replaces them with `registerActionAsync` and `addTriggerActionAsync` * Makes all registry consumption methods async, such as `getAction`, `getTriggerActions` and `getFrequentlyChangingActionsForTrigger` PR updates presentation_panel plugin to use `registerActionAsync`. With actions behind async import, page load bundle size has been reduced by 21.1KB. <img width="500" alt="Screenshot 2025-01-08 at 2 14 23 PM" src="https://github.com/user-attachments/assets/34a2cae9-dc5e-429b-bbdb-ffd9dfe1cce3" /> Also, async exports are [contained in a single file `panel_module`](elastic#206117). This results in dashboard only loading one `presentationPanel.chunk`. <img width="500" alt="Screenshot 2025-01-08 at 2 15 02 PM" src="https://github.com/user-attachments/assets/e083b852-b50d-4fa7-8ebd-e2f56f85e998" /> --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
#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. --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Closes #191642, part of #205512
PR makes the following changes to uiActions API.
registerActionandaddTriggerActionand replaces them withregisterActionAsyncandaddTriggerActionAsyncgetAction,getTriggerActionsandgetFrequentlyChangingActionsForTriggerPR updates presentation_panel plugin to use
registerActionAsync. With actions behind async import, page load bundle size has been reduced by 21.1KB.Also, async exports are contained in a single file
panel_module. This results in dashboard only loading onepresentationPanel.chunk.