Skip to content

Commit

Permalink
[Lens] Remove <NativeRenderer /> (#161521)
Browse files Browse the repository at this point in the history 
## Summary

The NativeRenderer component is currently used to mount another
component in a separate mounting point. As far as I recall, we
introduced <NativeRenderer/> to allow users to create visualizations in
non-React frameworks. The idea was that users could write their own Lens
visualizations or datasources code and integrate it with our system.
However, it seems that this concept hasn't gained traction and we don’t
have it prioritized. Even if users express interest in writing their
visualizations outside of React, it is still possible to do so with some
additional boilerplate code (which we could provide as an example
non-React visualization).
Pros:

1. Simplifies and shortens the code:
1.1. Testing and debugging become easier as we no longer need to check
separate React trees when integrating frame, data source, and
visualization components.
1.2. Components communicate using standard React patterns, making
maintenance and comprehension simpler.
1.3. Context providers no longer need to be passed to each separate
component since they are already within the context.
1.4. Easier propagation of events or any other form of inter-component
communication.

2. Greatly improves performance and facilitates maintenance:
2.1. Directly accessing context inside the DatasourcePanel eliminates
the need for context passing, resulting in better performance.
2.2. Removing the requirement for a separate React root also contributes
to improved performance.

3. The render method will be removed when we upgrade to React 18. While
we could replace it with the new createRoot method, it makes sense to
perform some cleanup ;)

Cons:
1. Setting up non-React visualization or data source code might become
slightly more complex.

Performance improvement for drag and drop action with these changes:

before:

<img width="1110" alt="Screenshot 2023-07-10 at 07 14 39"
src="https://github.com/elastic/kibana/assets/4283304/45a1b09b-5189-46f5-af2b-7781fcf4e774">

after:

<img width="1117" alt="Screenshot 2023-07-10 at 07 16 24"
src="https://github.com/elastic/kibana/assets/4283304/0e704da1-3220-4eb9-8fa0-cc3584a90090">

## Single render when dragging:

(the first image is 3 screenshots from 3 different react roots as they
have separate mounting point. The complete render time is ~380ms)
<img width="1117" alt="Screenshot 2023-07-10 at 07 16 24"
src="https://github.com/elastic/kibana/assets/4283304/6d7f2d9f-a758-476e-8efb-38693ae90097">

After we have one common render tree. Because we don't have to pass
context down as a prop, we greatly reduced the number of components
rerendered. (I will be working on reducing the render time for workspace
panel as this seems to still be a bottleneck point)
<img width="732" alt="Screenshot 2023-07-10 at 14 52 41"
src="https://github.com/elastic/kibana/assets/4283304/03ec97b3-8225-490e-8884-0fd4e69587e8">

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
@mbondyra @kibanamachine
mbondyra and kibanamachine authored Jul 11, 2023
1 parent 5d06694 commit 95e5087
Show file tree
Hide file tree
Showing 33 changed files with 424 additions and 1,196 deletions.
3 changes: 0 additions & 3 deletions x-pack/examples/third_party_vis_lens_example/kibana.jsonc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
"developerExamples",
"expressions",
"fieldFormats"
],
"requiredBundles": [
"kibanaReact"
]
}
}
25 changes: 10 additions & 15 deletions x-pack/examples/third_party_vis_lens_example/public/visualization.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@

import React from 'react';
import { EuiFormRow, EuiColorPicker } from '@elastic/eui';
import { render } from 'react-dom';
import { Ast } from '@kbn/interpreter';
import { ThemeServiceStart } from '@kbn/core/public';
import { KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { Visualization, OperationMetadata } from '@kbn/lens-plugin/public';
import { layerTypes } from '@kbn/lens-plugin/public';
import type { RotatingNumberState } from '../common/types';
Expand Down Expand Up @@ -166,19 +164,16 @@ export const getRotatingNumberVisualization = ({
return { ...prevState, accessor: undefined };
},

renderDimensionEditor(domElement, props) {
render(
<KibanaThemeProvider theme$={theme.theme$}>
<EuiFormRow label="Pick a color">
<EuiColorPicker
onChange={(newColor) => {
props.setState({ ...props.state, color: newColor });
}}
color={props.state.color}
/>
</EuiFormRow>
</KibanaThemeProvider>,
domElement
DimensionEditorComponent(props) {
return (
<EuiFormRow label="Pick a color">
<EuiColorPicker
onChange={(newColor) => {
props.setState({ ...props.state, color: newColor });
}}
color={props.state.color}
/>
</EuiFormRow>
);
},
});
1 change: 0 additions & 1 deletion x-pack/examples/third_party_vis_lens_example/tsconfig.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
"kbn_references": [
"@kbn/core",
"@kbn/expressions-plugin",
"@kbn/kibana-react-plugin",
"@kbn/data-views-plugin",
"@kbn/field-formats-plugin",
"@kbn/lens-plugin",
Expand Down
2 changes: 1 addition & 1 deletion .../plugins/lens/public/app_plugin/shared/edit_on_the_fly/lens_configuration_flyout.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ describe('LensEditConfigurationFlyout', () => {
const { instance } = await prepareAndMountComponent(props);
expect(instance.find(VisualizationToolbar).prop('activeVisualization')).toMatchInlineSnapshot(`
Object {
"DimensionEditorComponent": [MockFunction],
"appendLayer": [MockFunction],
"clearLayer": [MockFunction],
"getConfiguration": [MockFunction] {
Expand Down Expand Up @@ -414,7 +415,6 @@ describe('LensEditConfigurationFlyout', () => {
"initialize": [MockFunction],
"removeDimension": [MockFunction],
"removeLayer": [MockFunction],
"renderDimensionEditor": [MockFunction],
"setDimension": [MockFunction],
"switchVisualizationType": [MockFunction],
"toExpression": [MockFunction],
Expand Down
4 changes: 0 additions & 4 deletions x-pack/plugins/lens/public/datasources/form_based/datapanel.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import React from 'react';
import ReactDOM from 'react-dom';
import { createMockedDragDropContext } from '../../mocks';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';
import {
dataViewPluginMocks,
Expand Down Expand Up @@ -408,9 +407,6 @@ describe('FormBased Data Panel', () => {
dataViews,
}),
setState: jest.fn(),
dragDropContext: createMockedDragDropContext({
dragging: { id: '1', humanData: { label: 'Label' } },
}),
dateRange: { fromDate: '2019-01-01', toDate: '2020-01-01' },
frame: getFrameAPIMock({
indexPatterns: indexPatterns as unknown as DataViewsState['indexPatterns'],
Expand Down
209 changes: 49 additions & 160 deletions x-pack/plugins/lens/public/datasources/form_based/form_based.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
*/

import React from 'react';
import { render } from 'react-dom';
import { I18nProvider } from '@kbn/i18n-react';
import type { CoreStart, SavedObjectReference } from '@kbn/core/public';
import { i18n } from '@kbn/i18n';
import { TimeRange } from '@kbn/es-query';
Expand All @@ -16,7 +14,6 @@ import type { FieldFormatsStart } from '@kbn/field-formats-plugin/public';
import { flatten, isEqual } from 'lodash';
import type { DataViewsPublicPluginStart, DataView } from '@kbn/data-views-plugin/public';
import type { IndexPatternFieldEditorStart } from '@kbn/data-view-field-editor-plugin/public';
import { KibanaContextProvider, KibanaThemeProvider } from '@kbn/kibana-react-plugin/public';
import { DataPublicPluginStart, UI_SETTINGS } from '@kbn/data-plugin/public';
import { VisualizeFieldContext } from '@kbn/ui-actions-plugin/public';
import { ChartsPluginSetup } from '@kbn/charts-plugin/public';
Expand All @@ -25,7 +22,7 @@ import { FormattedMessage } from '@kbn/i18n-react';
import type { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public';
import { EuiButton } from '@elastic/eui';
import type { SharePluginStart } from '@kbn/share-plugin/public';
import { ChildDragDropProvider, type DraggingIdentifier } from '@kbn/dom-drag-drop';
import { type DraggingIdentifier } from '@kbn/dom-drag-drop';
import { DimensionTrigger } from '@kbn/visualization-ui-components/public';
import memoizeOne from 'memoize-one';
import type {
Expand Down Expand Up @@ -191,7 +188,7 @@ export function getFormBasedDatasource({
dataViewFieldEditor: IndexPatternFieldEditorStart;
uiActions: UiActionsStart;
}) {
const { uiSettings, settings } = core;
const { uiSettings } = core;

const DATASOURCE_ID = 'formBased';
const ALIAS_IDS = ['indexpattern'];
Expand Down Expand Up @@ -452,68 +449,27 @@ export function getFormBasedDatasource({
searchSessionId
),

renderLayerSettings(domElement, props) {
render(
<KibanaThemeProvider theme$={core.theme.theme$}>
<I18nProvider>
<KibanaContextProvider
services={{
...core,
data,
dataViews,
fieldFormats,
charts,
unifiedSearch,
share,
}}
>
<LayerSettingsPanel {...props} />
</KibanaContextProvider>
</I18nProvider>
</KibanaThemeProvider>,
domElement
);
LayerSettingsComponent(props) {
return <LayerSettingsPanel {...props} />;
},

renderDataPanel(domElement: Element, props: DatasourceDataPanelProps<FormBasedPrivateState>) {
const { onChangeIndexPattern, dragDropContext, ...otherProps } = props;
DataPanelComponent(props: DatasourceDataPanelProps<FormBasedPrivateState>) {
const { onChangeIndexPattern, ...otherProps } = props;
const layerFields = formBasedDatasource?.getSelectedFields?.(props.state);

render(
<KibanaThemeProvider theme$={core.theme.theme$}>
<I18nProvider>
<KibanaContextProvider
services={{
...core,
data,
dataViews,
fieldFormats,
charts,
unifiedSearch,
share,
}}
>
<ChildDragDropProvider value={dragDropContext}>
<FormBasedDataPanel
data={data}
dataViews={dataViews}
fieldFormats={fieldFormats}
charts={charts}
indexPatternFieldEditor={dataViewFieldEditor}
{...otherProps}
core={core}
uiActions={uiActions}
onIndexPatternRefresh={onRefreshIndexPattern}
layerFields={layerFields}
/>
</ChildDragDropProvider>
</KibanaContextProvider>
</I18nProvider>
</KibanaThemeProvider>,
domElement
return (
<FormBasedDataPanel
data={data}
dataViews={dataViews}
fieldFormats={fieldFormats}
charts={charts}
indexPatternFieldEditor={dataViewFieldEditor}
{...otherProps}
core={core}
uiActions={uiActions}
onIndexPatternRefresh={onRefreshIndexPattern}
layerFields={layerFields}
/>
);
},

uniqueLabels(state: FormBasedPrivateState, indexPatternsMap: IndexPatternMap) {
const layers = state.layers;
const columnLabelMap = {} as Record<string, string>;
Expand All @@ -540,115 +496,48 @@ export function getFormBasedDatasource({
return columnLabelMap;
},

renderDimensionTrigger: (
domElement: Element,
props: DatasourceDimensionTriggerProps<FormBasedPrivateState>
) => {
DimensionTriggerComponent: (props: DatasourceDimensionTriggerProps<FormBasedPrivateState>) => {
const columnLabelMap = formBasedDatasource.uniqueLabels(props.state, props.indexPatterns);
const uniqueLabel = columnLabelMap[props.columnId];
const formattedLabel = wrapOnDot(uniqueLabel);

render(
<KibanaThemeProvider theme$={core.theme.theme$}>
<I18nProvider>
<KibanaContextProvider
services={{
appName: 'lens',
storage,
uiSettings,
settings,
data,
fieldFormats,
savedObjects: core.savedObjects,
docLinks: core.docLinks,
unifiedSearch,
}}
>
<DimensionTrigger id={props.columnId} label={formattedLabel} />
</KibanaContextProvider>
</I18nProvider>
</KibanaThemeProvider>,
domElement
);
return <DimensionTrigger id={props.columnId} label={formattedLabel} />;
},

renderDimensionEditor: (
domElement: Element,
props: DatasourceDimensionEditorProps<FormBasedPrivateState>
) => {
DimensionEditorComponent: (props: DatasourceDimensionEditorProps<FormBasedPrivateState>) => {
const columnLabelMap = formBasedDatasource.uniqueLabels(props.state, props.indexPatterns);

render(
<KibanaThemeProvider theme$={core.theme.theme$}>
<I18nProvider>
<KibanaContextProvider
services={{
appName: 'lens',
storage,
uiSettings,
settings,
data,
fieldFormats,
savedObjects: core.savedObjects,
docLinks: core.docLinks,
http: core.http,
unifiedSearch,
}}
>
<FormBasedDimensionEditor
uiSettings={uiSettings}
storage={storage}
fieldFormats={fieldFormats}
http={core.http}
data={data}
unifiedSearch={unifiedSearch}
dataViews={dataViews}
uniqueLabel={columnLabelMap[props.columnId]}
notifications={core.notifications}
{...props}
/>
</KibanaContextProvider>
</I18nProvider>
</KibanaThemeProvider>,
domElement
return (
<FormBasedDimensionEditor
uiSettings={uiSettings}
storage={storage}
fieldFormats={fieldFormats}
http={core.http}
data={data}
unifiedSearch={unifiedSearch}
dataViews={dataViews}
uniqueLabel={columnLabelMap[props.columnId]}
notifications={core.notifications}
{...props}
/>
);
},

renderLayerPanel: (
domElement: Element,
props: DatasourceLayerPanelProps<FormBasedPrivateState>
) => {
LayerPanelComponent: (props: DatasourceLayerPanelProps<FormBasedPrivateState>) => {
const { onChangeIndexPattern, ...otherProps } = props;
render(
<KibanaThemeProvider theme$={core.theme.theme$}>
<I18nProvider>
<KibanaContextProvider
services={{
...core,
data,
dataViews,
fieldFormats,
charts,
unifiedSearch,
share,
}}
>
<LayerPanel
onChangeIndexPattern={(indexPatternId) => {
triggerActionOnIndexPatternChange({
indexPatternId,
state: props.state,
layerId: props.layerId,
uiActions,
});
onChangeIndexPattern(indexPatternId, DATASOURCE_ID, props.layerId);
}}
{...otherProps}
/>
</KibanaContextProvider>
</I18nProvider>
</KibanaThemeProvider>,
domElement
return (
<LayerPanel
onChangeIndexPattern={(indexPatternId) => {
triggerActionOnIndexPatternChange({
indexPatternId,
state: props.state,
layerId: props.layerId,
uiActions,
});
onChangeIndexPattern(indexPatternId, DATASOURCE_ID, props.layerId);
}}
{...otherProps}
/>
);
},

Expand Down
3 changes: 1 addition & 2 deletions x-pack/plugins/lens/public/datasources/text_based/datapanel.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { mountWithIntl } from '@kbn/test-jest-helpers';

import { uiActionsPluginMock } from '@kbn/ui-actions-plugin/public/mocks';
import { createIndexPatternServiceMock } from '../../mocks/data_views_service_mock';
import { createMockFramePublicAPI, createMockedDragDropContext } from '../../mocks';
import { createMockFramePublicAPI } from '../../mocks';
import { DataViewsState } from '../../state_management';

const fieldsFromQuery = [
Expand Down Expand Up @@ -184,7 +184,6 @@ describe('TextBased Query Languages Data Panel', () => {
])
),
},
dragDropContext: createMockedDragDropContext(),
core,
dateRange: {
fromDate: 'now-7d',
Expand Down
Loading

0 comments on commit 95e5087

Please sign in to comment.