[controls] fix controls plugin loading async chunks on home page (#19…

…4182) Closes #194180 PR resolves issue by updating registry to store getFactory instead of the results of getFactory. In this way, `getFactory` is not executed until control is rendered and bundle is needed. --------- Co-authored-by: kibanamachine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
elastic · Sep 26, 2024 · 98b8df7 · 98b8df7
1 parent 8bd405a
commit 98b8df7
Show file tree

Hide file tree

Showing 6 changed files with 153 additions and 91 deletions.
diff --git a/src/plugins/controls/public/control_factory_registry.ts b/src/plugins/controls/public/control_factory_registry.ts
@@ -10,9 +10,9 @@
 import { i18n } from '@kbn/i18n';
 import { ControlFactory, DefaultControlApi } from './controls/types';
 
-const registry: { [key: string]: ControlFactory<any, any> } = {};
+const registry: { [key: string]: () => Promise<ControlFactory<any, any>> } = {};
 
-export const registerControlFactory = async <
+export const registerControlFactory = <
   State extends object = object,
   ApiType extends DefaultControlApi = DefaultControlApi
 >(
@@ -26,23 +26,23 @@ export const registerControlFactory = async <
         values: { key: type },
       })
     );
-  registry[type] = (await getFactory()) as ControlFactory<any, any>;
+  registry[type] = getFactory as () => Promise<ControlFactory<any, any>>;
 };
 
-export const getControlFactory = <
+export const getControlFactory = async <
   State extends object = object,
   ApiType extends DefaultControlApi = DefaultControlApi
 >(
   key: string
-): ControlFactory<State, ApiType> => {
+): Promise<ControlFactory<State, ApiType>> => {
   if (registry[key] === undefined)
     throw new Error(
       i18n.translate('controls.controlFactoryRegistry.factoryNotFoundError', {
         defaultMessage: 'No control factory found for type: {key}',
         values: { key },
       })
     );
-  return registry[key] as ControlFactory<State, ApiType>;
+  return (await registry[key]()) as ControlFactory<State, ApiType>;
 };
 
 export const getAllControlTypes = () => {

diff --git a/src/plugins/controls/public/control_group/components/control_renderer.tsx b/src/plugins/controls/public/control_group/components/control_renderer.tsx
@@ -50,7 +50,7 @@ export const ControlRenderer = <
 
       async function buildControl() {
         const parentApi = getParentApi();
-        const factory = getControlFactory<StateType, ApiType>(type);
+        const factory = await getControlFactory<StateType, ApiType>(type);
         const buildApi = (
           apiRegistration: ControlApiRegistration<ApiType>,
           comparators: StateComparators<StateType>

diff --git a/src/plugins/controls/public/controls/data_controls/data_control_editor.test.tsx b/src/plugins/controls/public/controls/data_controls/data_control_editor.test.tsx
@@ -117,15 +117,17 @@ describe('Data control editor', () => {
     return controlEditor.getByTestId(testId).getAttribute('aria-pressed');
   };
 
-  const mockRegistry: { [key: string]: ControlFactory<DefaultDataControlState, DataControlApi> } = {
-    search: getMockedSearchControlFactory({ parentApi: controlGroupApi }),
-    optionsList: getMockedOptionsListControlFactory({ parentApi: controlGroupApi }),
-    rangeSlider: getMockedRangeSliderControlFactory({ parentApi: controlGroupApi }),
+  const mockRegistry: {
+    [key: string]: () => Promise<ControlFactory<DefaultDataControlState, DataControlApi>>;
+  } = {
+    search: async () => getMockedSearchControlFactory({ parentApi: controlGroupApi }),
+    optionsList: async () => getMockedOptionsListControlFactory({ parentApi: controlGroupApi }),
+    rangeSlider: async () => getMockedRangeSliderControlFactory({ parentApi: controlGroupApi }),
   };
 
   beforeAll(() => {
     (getAllControlTypes as jest.Mock).mockReturnValue(Object.keys(mockRegistry));
-    (getControlFactory as jest.Mock).mockImplementation((key) => mockRegistry[key]);
+    (getControlFactory as jest.Mock).mockImplementation((key) => mockRegistry[key]());
   });
 
   describe('creating a new control', () => {
@@ -146,33 +148,35 @@ describe('Data control editor', () => {
 
     test('CompatibleControlTypesComponent respects ordering', async () => {
       const tempRegistry: {
-        [key: string]: ControlFactory<DefaultDataControlState, DataControlApi>;
+        [key: string]: () => Promise<ControlFactory<DefaultDataControlState, DataControlApi>>;
       } = {
         ...mockRegistry,
-        alphabeticalFirstControl: {
-          type: 'alphabeticalFirst',
-          getIconType: () => 'lettering',
-          getDisplayName: () => 'Alphabetically first',
-          isFieldCompatible: () => true,
-          buildControl: jest.fn().mockReturnValue({
-            api: controlGroupApi,
-            Component: <>Should be first alphabetically</>,
-          }),
-        } as DataControlFactory,
-        supremeControl: {
-          type: 'supremeControl',
-          order: 100, // force it first despite alphabetical ordering
-          getIconType: () => 'starFilled',
-          getDisplayName: () => 'Supreme leader',
-          isFieldCompatible: () => true,
-          buildControl: jest.fn().mockReturnValue({
-            api: controlGroupApi,
-            Component: <>This control is forced first via the factory order</>,
-          }),
-        } as DataControlFactory,
+        alphabeticalFirstControl: async () =>
+          ({
+            type: 'alphabeticalFirst',
+            getIconType: () => 'lettering',
+            getDisplayName: () => 'Alphabetically first',
+            isFieldCompatible: () => true,
+            buildControl: jest.fn().mockReturnValue({
+              api: controlGroupApi,
+              Component: <>Should be first alphabetically</>,
+            }),
+          } as DataControlFactory),
+        supremeControl: async () =>
+          ({
+            type: 'supremeControl',
+            order: 100, // force it first despite alphabetical ordering
+            getIconType: () => 'starFilled',
+            getDisplayName: () => 'Supreme leader',
+            isFieldCompatible: () => true,
+            buildControl: jest.fn().mockReturnValue({
+              api: controlGroupApi,
+              Component: <>This control is forced first via the factory order</>,
+            }),
+          } as DataControlFactory),
       };
       (getAllControlTypes as jest.Mock).mockReturnValue(Object.keys(tempRegistry));
-      (getControlFactory as jest.Mock).mockImplementation((key) => tempRegistry[key]);
+      (getControlFactory as jest.Mock).mockImplementation((key) => tempRegistry[key]());
 
       const controlEditor = await mountComponent({});
       const menu = controlEditor.getByTestId('controlTypeMenu');
@@ -185,7 +189,7 @@ describe('Data control editor', () => {
       expect(menu.children[4].textContent).toEqual('Search');
 
       (getAllControlTypes as jest.Mock).mockReturnValue(Object.keys(mockRegistry));
-      (getControlFactory as jest.Mock).mockImplementation((key) => mockRegistry[key]);
+      (getControlFactory as jest.Mock).mockImplementation((key) => mockRegistry[key]());
     });
 
     test('selecting a keyword field - can only create an options list control', async () => {

diff --git a/src/plugins/controls/public/controls/data_controls/data_control_editor.tsx b/src/plugins/controls/public/controls/data_controls/data_control_editor.tsx
@@ -7,7 +7,7 @@
  * License v3.0 only", or the "Server Side Public License, v 1".
  */
 
-import React, { useMemo, useState } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import useAsync from 'react-use/lib/useAsync';
 
 import {
@@ -27,6 +27,7 @@ import {
   EuiIcon,
   EuiKeyPadMenu,
   EuiKeyPadMenuItem,
+  EuiSkeletonRectangle,
   EuiSpacer,
   EuiSwitch,
   EuiTitle,
@@ -39,6 +40,7 @@ import {
   withSuspense,
 } from '@kbn/presentation-util-plugin/public';
 
+import { asyncMap } from '@kbn/std';
 import {
   DEFAULT_CONTROL_GROW,
   DEFAULT_CONTROL_WIDTH,
@@ -56,6 +58,7 @@ import {
   type DataControlFactory,
   type DataControlFieldRegistry,
 } from './types';
+import { ControlFactory } from '../types';
 
 export interface ControlEditorProps<
   State extends DefaultDataControlState = DefaultDataControlState
@@ -83,60 +86,89 @@ const CompatibleControlTypesComponent = ({
   selectedControlType?: string;
   setSelectedControlType: (type: string) => void;
 }) => {
-  const dataControlFactories = useMemo(() => {
-    return getAllControlTypes()
-      .map((type) => getControlFactory(type))
-      .filter((factory) => isDataControlFactory(factory))
-      .sort(
-        (
-          { order: orderA = 0, getDisplayName: getDisplayNameA },
-          { order: orderB = 0, getDisplayName: getDisplayNameB }
-        ) => {
-          const orderComparison = orderB - orderA; // sort descending by order
-          return orderComparison === 0
-            ? getDisplayNameA().localeCompare(getDisplayNameB()) // if equal order, compare display names
-            : orderComparison;
+  const [dataControlFactories, setDataControlFactories] = useState<
+    DataControlFactory[] | undefined
+  >(undefined);
+
+  useEffect(() => {
+    let cancelled = false;
+
+    asyncMap<string, ControlFactory>(getAllControlTypes(), async (controlType) =>
+      getControlFactory(controlType)
+    )
+      .then((controlFactories) => {
+        if (!cancelled) {
+          setDataControlFactories(
+            controlFactories
+              .filter((factory) => isDataControlFactory(factory))
+              .sort(
+                (
+                  { order: orderA = 0, getDisplayName: getDisplayNameA },
+                  { order: orderB = 0, getDisplayName: getDisplayNameB }
+                ) => {
+                  const orderComparison = orderB - orderA; // sort descending by order
+                  return orderComparison === 0
+                    ? getDisplayNameA().localeCompare(getDisplayNameB()) // if equal order, compare display names
+                    : orderComparison;
+                }
+              ) as unknown as DataControlFactory[]
+          );
         }
-      );
+      })
+      .catch(() => {
+        if (!cancelled) setDataControlFactories([]);
+      });
+
+    return () => {
+      cancelled = true;
+    };
   }, []);
 
   return (
-    <EuiKeyPadMenu data-test-subj={`controlTypeMenu`} aria-label={'type'}>
-      {dataControlFactories.map((factory) => {
-        const disabled =
-          fieldRegistry && selectedFieldName
-            ? !fieldRegistry[selectedFieldName]?.compatibleControlTypes.includes(factory.type)
-            : true;
-        const keyPadMenuItem = (
-          <EuiKeyPadMenuItem
-            key={factory.type}
-            id={`create__${factory.type}`}
-            aria-label={factory.getDisplayName()}
-            data-test-subj={`create__${factory.type}`}
-            isSelected={factory.type === selectedControlType}
-            disabled={disabled}
-            onClick={() => setSelectedControlType(factory.type)}
-            label={factory.getDisplayName()}
-          >
-            <EuiIcon type={factory.getIconType()} size="l" />
-          </EuiKeyPadMenuItem>
-        );
+    <EuiSkeletonRectangle
+      isLoading={dataControlFactories === undefined}
+      width="100px"
+      height="100px"
+    >
+      <EuiKeyPadMenu data-test-subj={`controlTypeMenu`} aria-label={'type'}>
+        {(dataControlFactories ?? []).map((factory) => {
+          const disabled =
+            fieldRegistry && selectedFieldName
+              ? !fieldRegistry[selectedFieldName]?.compatibleControlTypes.includes(factory.type)
+              : true;
+          const keyPadMenuItem = (
+            <EuiKeyPadMenuItem
+              key={factory.type}
+              id={`create__${factory.type}`}
+              aria-label={factory.getDisplayName()}
+              data-test-subj={`create__${factory.type}`}
+              isSelected={factory.type === selectedControlType}
+              disabled={disabled}
+              onClick={() => setSelectedControlType(factory.type)}
+              label={factory.getDisplayName()}
+            >
+              <EuiIcon type={factory.getIconType()} size="l" />
+            </EuiKeyPadMenuItem>
+          );
 
-        return disabled ? (
-          <EuiToolTip
-            key={`disabled__${factory.type}`}
-            content={DataControlEditorStrings.manageControl.dataSource.getControlTypeErrorMessage({
-              fieldSelected: Boolean(selectedFieldName),
-              controlType: factory.getDisplayName(),
-            })}
-          >
-            {keyPadMenuItem}
-          </EuiToolTip>
-        ) : (
-          keyPadMenuItem
-        );
-      })}
-    </EuiKeyPadMenu>
+          return disabled ? (
+            <EuiToolTip
+              key={`disabled__${factory.type}`}
+              content={DataControlEditorStrings.manageControl.dataSource.getControlTypeErrorMessage(
+                {
+                  fieldSelected: Boolean(selectedFieldName),
+                  controlType: factory.getDisplayName(),
+                }
+              )}
+            >
+              {keyPadMenuItem}
+            </EuiToolTip>
+          ) : (
+            keyPadMenuItem
+          );
+        })}
+      </EuiKeyPadMenu>
+    </EuiSkeletonRectangle>
   );
 };
 
@@ -186,9 +218,33 @@ export const DataControlEditor = <State extends DefaultDataControlState = Defaul
     };
   }, [editorState.dataViewId]);
 
+  const [controlFactory, setControlFactory] = useState<DataControlFactory | undefined>(undefined);
+  useEffect(() => {
+    if (!selectedControlType) {
+      setControlFactory(undefined);
+      return;
+    }
+
+    let cancelled = false;
+    getControlFactory(selectedControlType)
+      .then((nextControlFactory) => {
+        if (!cancelled) {
+          setControlFactory(nextControlFactory as unknown as DataControlFactory);
+        }
+      })
+      .catch(() => {
+        if (!cancelled) {
+          setControlFactory(undefined);
+        }
+      });
+
+    return () => {
+      cancelled = true;
+    };
+  }, [selectedControlType]);
+
   const CustomSettingsComponent = useMemo(() => {
-    if (!selectedControlType || !editorState.fieldName || !fieldRegistry) return;
-    const controlFactory = getControlFactory(selectedControlType) as DataControlFactory;
+    if (!controlFactory || !editorState.fieldName || !fieldRegistry) return;
     const CustomSettings = controlFactory.CustomOptionsComponent;
 
     if (!CustomSettings) return;
@@ -217,7 +273,7 @@ export const DataControlEditor = <State extends DefaultDataControlState = Defaul
         />
       </EuiDescribedFormGroup>
     );
-  }, [fieldRegistry, selectedControlType, initialState, editorState, controlGroupApi]);
+  }, [fieldRegistry, controlFactory, initialState, editorState, controlGroupApi]);
 
   return (
     <>

diff --git a/src/plugins/controls/public/controls/data_controls/data_control_editor_utils.ts b/src/plugins/controls/public/controls/data_controls/data_control_editor_utils.ts
@@ -10,6 +10,7 @@
 import { memoize } from 'lodash';
 
 import type { DataView } from '@kbn/data-views-plugin/common';
+import { asyncMap } from '@kbn/std';
 import { getAllControlTypes, getControlFactory } from '../../control_factory_registry';
 import { isDataControlFactory, type DataControlFieldRegistry } from './types';
 
@@ -23,7 +24,7 @@ export const getDataControlFieldRegistry = memoize(
 const loadFieldRegistryFromDataView = async (
   dataView: DataView
 ): Promise<DataControlFieldRegistry> => {
-  const controlFactories = getAllControlTypes().map((controlType) =>
+  const controlFactories = await asyncMap(getAllControlTypes(), async (controlType) =>
     getControlFactory(controlType)
   );
   const fieldRegistry: DataControlFieldRegistry = {};

diff --git a/src/plugins/controls/tsconfig.json b/src/plugins/controls/tsconfig.json
@@ -37,7 +37,8 @@
     "@kbn/content-management-utils",
     "@kbn/field-formats-plugin",
     "@kbn/presentation-panel-plugin",
-    "@kbn/shared-ux-utility"
+    "@kbn/shared-ux-utility",
+    "@kbn/std"
   ],
   "exclude": ["target/**/*"]
 }