From 4475357171e091ca7c81817673d5afb6261b92fd Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Thu, 26 Sep 2024 16:12:38 -0600 Subject: [PATCH] [controls] fix controls plugin loading async chunks on home page (#194182) Closes https://github.com/elastic/kibana/issues/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 <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Elastic Machine (cherry picked from commit 98b8df7253b8450b16e06c0355d67425fd772c3c) --- .../public/control_factory_registry.ts | 12 +- .../components/control_renderer.tsx | 2 +- .../data_control_editor.test.tsx | 62 +++---- .../data_controls/data_control_editor.tsx | 162 ++++++++++++------ .../data_control_editor_utils.ts | 3 +- src/plugins/controls/tsconfig.json | 3 +- 6 files changed, 153 insertions(+), 91 deletions(-) diff --git a/src/plugins/controls/public/control_factory_registry.ts b/src/plugins/controls/public/control_factory_registry.ts index 40ded8cbcbf00..35488cf3f7661 100644 --- 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 } = {}; +const registry: { [key: string]: () => Promise> } = {}; -export const registerControlFactory = async < +export const registerControlFactory = < State extends object = object, ApiType extends DefaultControlApi = DefaultControlApi >( @@ -26,15 +26,15 @@ export const registerControlFactory = async < values: { key: type }, }) ); - registry[type] = (await getFactory()) as ControlFactory; + registry[type] = getFactory as () => Promise>; }; -export const getControlFactory = < +export const getControlFactory = async < State extends object = object, ApiType extends DefaultControlApi = DefaultControlApi >( key: string -): ControlFactory => { +): Promise> => { if (registry[key] === undefined) throw new Error( i18n.translate('controls.controlFactoryRegistry.factoryNotFoundError', { @@ -42,7 +42,7 @@ export const getControlFactory = < values: { key }, }) ); - return registry[key] as ControlFactory; + return (await registry[key]()) as ControlFactory; }; 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 index a30c39f1dfe2a..181160fae4d13 100644 --- 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(type); + const factory = await getControlFactory(type); const buildApi = ( apiRegistration: ControlApiRegistration, comparators: StateComparators 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 index 23f9b053d23b2..e1ae7a9bfd769 100644 --- 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 } = { - search: getMockedSearchControlFactory({ parentApi: controlGroupApi }), - optionsList: getMockedOptionsListControlFactory({ parentApi: controlGroupApi }), - rangeSlider: getMockedRangeSliderControlFactory({ parentApi: controlGroupApi }), + const mockRegistry: { + [key: string]: () => Promise>; + } = { + 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; + [key: string]: () => Promise>; } = { ...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 index 43d0f46324557..23fd95978ff82 100644 --- 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(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 ( - - {dataControlFactories.map((factory) => { - const disabled = - fieldRegistry && selectedFieldName - ? !fieldRegistry[selectedFieldName]?.compatibleControlTypes.includes(factory.type) - : true; - const keyPadMenuItem = ( - setSelectedControlType(factory.type)} - label={factory.getDisplayName()} - > - - - ); + + + {(dataControlFactories ?? []).map((factory) => { + const disabled = + fieldRegistry && selectedFieldName + ? !fieldRegistry[selectedFieldName]?.compatibleControlTypes.includes(factory.type) + : true; + const keyPadMenuItem = ( + setSelectedControlType(factory.type)} + label={factory.getDisplayName()} + > + + + ); - return disabled ? ( - - {keyPadMenuItem} - - ) : ( - keyPadMenuItem - ); - })} - + return disabled ? ( + + {keyPadMenuItem} + + ) : ( + keyPadMenuItem + ); + })} + + ); }; @@ -186,9 +218,33 @@ export const DataControlEditor = (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 = ); - }, [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 index 7bf127462770a..d4d0125bd1479 100644 --- 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 => { - 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 index abcafa291358e..e1040faecc1b0 100644 --- 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/**/*"] }