[Share] Convert Share registry to async registry by nickpeihl · Pull Request #224139 · elastic/kibana

nickpeihl · 2025-06-16T20:24:23Z

WIP Currently not wired up to internal share contexts, but I've hit a wall trying to asynchronously load the menu items. Feel free to pick up where I left off or I can continue to work on it as I have time.

Summary

Changes the Share registry in the share plugin to an async registry.

With this PR:

The arguments to the registerShareIntegration method on ShareRegistry require a shareObject, key, and a function returning a Promise that returns the ShareIntegration.
availableIntegrations and resolveShareItemsForShareContext now return promises.
Plugins register their integrations with a function returning a Promise. This allows lazy imports of modules which decreases the initial page load.

TODO

Update the ShareContext to asynchronously get the menuItems from the resolveShareItemsForShareContext promise.
Update plugin usage of availableIntegrations to support promises.

Preliminary initial page load decreases

I used node scripts/build_kibana_platform_plugins --profile --no-cache --dist to generate metrics.json files for all plugins in this branch and the main branch. And then manually compared the sizes in metrics.json for each plugin against the other branch.

plugin	main	this branch
reporting	52KB	44KB
lens	61KB	59KB

Reference

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

See some risk examples
...

Currently not wired up to internal share contexts

elasticmachine · 2025-06-16T20:24:38Z

🤖 Jobs for this PR can be triggered through checkboxes. 🚧

ℹ️ To trigger the CI, please tick the checkbox below 👇

Click to trigger kibana-pull-request for this PR!
Click to trigger kibana-deploy-project-from-pr for this PR!
Click to trigger kibana-deploy-cloud-from-pr for this PR!

elasticmachine · 2025-06-16T20:38:59Z

💔 Build Failed

Buildkite Build
Commit: 1e4c7f7

Failed CI Steps

Check Types

History

nickpeihl · 2025-06-17T18:50:20Z

+  const [hasShareIntegration, setHasShareIntegration] = useState(false);
+
  useEffect(() => {
+    let canceled = false;


Maybe it would be better to have a reusable custom hook that different plugins could use to check if their integrations are available?

We could create one such hook, however I'd rather this api remains synchronous, so we don't have a situation where the share options flicker because on the first render pass we can't tell if there are any available integrations.

nickpeihl · 2025-06-17T18:51:05Z

-    shareService?.availableIntegrations('dashboard', 'export')?.length
-  );
+  const [hasExportIntegration, setHasExportIntegration] = useState(false);
+  useEffect(() => {


See also https://github.com/elastic/kibana/pull/224139/files#r2152953864

nickpeihl · 2025-06-17T18:51:34Z

+import { useEffect, useState } from 'react';
+import type { DiscoverServices } from '../../../build_services';
+
+export function useHasShareIntegration({ share }: DiscoverServices) {


See also https://github.com/elastic/kibana/pull/224139/files#r2152953864

nickpeihl · 2025-06-17T18:54:25Z

-    shareActionIntent: ShareActionIntents
-  ): void {
+    key: ShareContextMapKey,
+    getShareActionIntent: () => Promise<ShareActionIntents>


Since the getShareActionIntent returns a promise, we can't introspect the shareActionIntent to generate a key. So we must specify a key in the parameters.

I think it might be better for this API we were to actually accept the groupId? for the integration and an id, so we can still manage the key for the integration internally based off these values that are meaningful to the consumer.

Another advantage to this approach would mean that we'll be able to check available integrations synchronously rather than async, and not have to modify functions returning nav config for other apps becoming async, with the resolution of the share implementation still async.

eokoneyo · 2025-07-07T09:04:19Z

+    shareObject: string,
+    key: ShareIntegrationMapKey,
+    getShareActionIntent: () => Promise<Omit<I, 'shareType'>>


This forces users to specify a share object, we should at the very least specify that shareObject is either * | string

eokoneyo · 2025-07-07T09:19:36Z

-    shareActionIntent: ShareActionIntents
-  ): void {
+    key: ShareContextMapKey,
+    getShareActionIntent: () => Promise<ShareActionIntents>


I think it might be better for this API we were to actually accept the groupId? for the integration and an id, so we can still manage the key for the integration internally based off these values that are meaningful to the consumer.

eokoneyo · 2025-07-07T09:25:57Z

  }

-  private resolveShareItemsForShareContext({
+  private async resolveShareItemsForShareContext({


Since this signature changed we'll need to in turn change the signature of toggleShareContextMenu to become async, because this method gets invoked within it. See 6a0319a

eokoneyo · 2025-07-07T09:28:25Z

+  const [hasShareIntegration, setHasShareIntegration] = useState(false);
+
  useEffect(() => {
+    let canceled = false;


We could create one such hook, however I'd rather this api remains synchronous, so we don't have a situation where the share options flicker because on the first render pass we can't tell if there are any available integrations.

eokoneyo · 2025-07-07T09:31:43Z

-    shareActionIntent: ShareActionIntents
-  ): void {
+    key: ShareContextMapKey,
+    getShareActionIntent: () => Promise<ShareActionIntents>


Another advantage to this approach would mean that we'll be able to check available integrations synchronously rather than async, and not have to modify functions returning nav config for other apps becoming async, with the resolution of the share implementation still async.

nickpeihl · 2025-08-11T17:03:56Z

Closing in preference to #228973

## Summary Informed from #224139 This PR makes changes to the share integration implementation, such that the definition for share config integration is a config that returns a promise. Adopting this approach allows checking the availability of a specific integration remains synchronous, however loading the implementation for any share integration would happen asynchronously, in turn allows us to have every share integration bundle to be defined as a separate chunk so we aren't loading every share integration render config up front but rather when it's been determined that the current user supports said integration.  --------- Co-authored-by: Nick Peihl <nick.peihl@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

## Summary Informed from elastic#224139 This PR makes changes to the share integration implementation, such that the definition for share config integration is a config that returns a promise. Adopting this approach allows checking the availability of a specific integration remains synchronous, however loading the implementation for any share integration would happen asynchronously, in turn allows us to have every share integration bundle to be defined as a separate chunk so we aren't loading every share integration render config up front but rather when it's been determined that the current user supports said integration.  --------- Co-authored-by: Nick Peihl <nick.peihl@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 63a0ed5) # Conflicts: # src/platform/packages/private/kbn-reporting/public/share/integrations/csv/csv_export_config.tsx # src/platform/packages/private/kbn-reporting/public/share/share_context_menu/register_pdf_png_modal_reporting.tsx # src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_discover_topnav.ts # src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_top_nav_links.tsx # src/platform/plugins/shared/share/public/services/share_menu_registry.test.ts # x-pack/platform/plugins/private/reporting/public/plugin.ts # x-pack/platform/plugins/shared/lens/public/app_plugin/csv_download_provider/csv_download_provider.tsx # x-pack/platform/plugins/shared/lens/public/plugin.ts

## Summary Informed from elastic#224139 This PR makes changes to the share integration implementation, such that the definition for share config integration is a config that returns a promise. Adopting this approach allows checking the availability of a specific integration remains synchronous, however loading the implementation for any share integration would happen asynchronously, in turn allows us to have every share integration bundle to be defined as a separate chunk so we aren't loading every share integration render config up front but rather when it's been determined that the current user supports said integration.  --------- Co-authored-by: Nick Peihl <nick.peihl@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 63a0ed5) # Conflicts: # src/platform/packages/private/kbn-reporting/public/share/integrations/csv/csv_export_config.tsx # src/platform/packages/private/kbn-reporting/public/share/share_context_menu/register_pdf_png_modal_reporting.tsx # src/platform/plugins/shared/share/public/services/share_menu_registry.test.ts # x-pack/platform/plugins/shared/lens/public/app_plugin/csv_download_provider/csv_download_provider.tsx # x-pack/platform/plugins/shared/lens/public/plugin.ts

## Summary Informed from elastic#224139 This PR makes changes to the share integration implementation, such that the definition for share config integration is a config that returns a promise. Adopting this approach allows checking the availability of a specific integration remains synchronous, however loading the implementation for any share integration would happen asynchronously, in turn allows us to have every share integration bundle to be defined as a separate chunk so we aren't loading every share integration render config up front but rather when it's been determined that the current user supports said integration.  --------- Co-authored-by: Nick Peihl <nick.peihl@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> (cherry picked from commit 63a0ed5) # Conflicts: # src/platform/packages/private/kbn-reporting/public/share/integrations/csv/csv_export_config.tsx # src/platform/packages/private/kbn-reporting/public/share/share_context_menu/register_pdf_png_modal_reporting.tsx # src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_discover_topnav.ts # src/platform/plugins/shared/discover/public/application/main/components/top_nav/use_top_nav_links.tsx # src/platform/plugins/shared/share/public/services/share_menu_registry.test.ts # x-pack/platform/plugins/private/reporting/public/plugin.ts # x-pack/platform/plugins/shared/lens/public/app_plugin/csv_download_provider/csv_download_provider.tsx # x-pack/platform/plugins/shared/lens/public/plugin.ts

## Summary Informed from elastic#224139 This PR makes changes to the share integration implementation, such that the definition for share config integration is a config that returns a promise. Adopting this approach allows checking the availability of a specific integration remains synchronous, however loading the implementation for any share integration would happen asynchronously, in turn allows us to have every share integration bundle to be defined as a separate chunk so we aren't loading every share integration render config up front but rather when it's been determined that the current user supports said integration.  --------- Co-authored-by: Nick Peihl <nick.peihl@elastic.co> Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Async share registry

1e4c7f7

Currently not wired up to internal share contexts

nickpeihl changed the title ~~Async share registry~~ [Share] Convert Share registry to async registry Jun 17, 2025

nickpeihl commented Jun 17, 2025

View reviewed changes

eokoneyo reviewed Jul 7, 2025

View reviewed changes

eokoneyo mentioned this pull request Jul 22, 2025

Async share registry #228973

Merged

nickpeihl closed this Aug 11, 2025

Conversation

nickpeihl commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

With this PR:

TODO

Preliminary initial page load decreases

Reference

Checklist

Identify risks

Uh oh!

elasticmachine commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💔 Build Failed

Failed CI Steps

History

Uh oh!

nickpeihl Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

eokoneyo Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickpeihl Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

nickpeihl Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

nickpeihl Jun 17, 2025

Choose a reason for hiding this comment

Uh oh!

eokoneyo Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

eokoneyo Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eokoneyo Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

eokoneyo Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

eokoneyo Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eokoneyo Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eokoneyo Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickpeihl commented Aug 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nickpeihl commented Jun 16, 2025 •

edited

Loading

elasticmachine commented Jun 16, 2025 •

edited

Loading

elasticmachine commented Jun 16, 2025 •

edited

Loading

eokoneyo Jul 7, 2025 •

edited

Loading

eokoneyo Jul 7, 2025 •

edited

Loading

eokoneyo Jul 7, 2025 •

edited

Loading

eokoneyo Jul 7, 2025 •

edited

Loading

eokoneyo Jul 7, 2025 •

edited

Loading