From 512859c98b908c8a5f57d3f5e671c39dd9bdf2e3 Mon Sep 17 00:00:00 2001 From: dej611 Date: Wed, 26 Oct 2022 12:43:50 +0200 Subject: [PATCH 1/3] :bug: Reduce rerendering on expression error --- .../render_complete_dispatcher.ts | 1 + .../lens/public/embeddable/embeddable.tsx | 11 +++++-- .../public/embeddable/expression_wrapper.tsx | 32 +++++++++++++++++-- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts b/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts index 7d6acf953d69b..f8fb9da67c901 100644 --- a/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts +++ b/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts @@ -38,6 +38,7 @@ export class RenderCompleteDispatcher { } public setEl(el?: HTMLElement) { + if (this.el !== el) { this.el = el; this.count = 0; if (el) this.dispatchInProgress(); diff --git a/x-pack/plugins/lens/public/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/embeddable/embeddable.tsx index 103c75844f816..34d6e837a1e22 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable.tsx @@ -493,7 +493,9 @@ export class Embeddable } onContainerStateChanged(containerState: LensEmbeddableInput) { - if (this.handleContainerStateChanged(containerState) || this.errors?.length) this.reload(); + if (this.handleContainerStateChanged(containerState)) { + this.reload(); + } } handleContainerStateChanged(containerState: LensEmbeddableInput): boolean { @@ -611,6 +613,10 @@ export class Embeddable }); }; + private onError: ExpressionWrapperProps['onExpressionError'] = () => { + this.renderComplete.dispatchError(); + }; + getExecutionContext() { if (this.savedVis) { const parentContext = this.parent?.getInput().executionContext || this.input.executionContext; @@ -651,7 +657,7 @@ export class Embeddable this.updateOutput({ ...this.getOutput(), loading: true, - error: undefined, + error: undefined, // Lens handles errors internally }); this.renderComplete.dispatchInProgress(); @@ -683,6 +689,7 @@ export class Embeddable style={input.style} executionContext={this.getExecutionContext()} canEdit={this.getIsEditable() && input.viewMode === 'edit'} + onExpressionError={this.onError} onRuntimeError={() => { this.logError('runtime'); }} diff --git a/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx b/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx index 3f10fba310b0c..f05d5599a7e57 100644 --- a/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx +++ b/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React from 'react'; +import React, { useRef } from 'react'; import { I18nProvider } from '@kbn/i18n-react'; import { FormattedMessage } from '@kbn/i18n-react'; import { EuiFlexGroup, EuiFlexItem, EuiText, EuiIcon, EuiEmptyPrompt } from '@elastic/eui'; @@ -45,6 +45,7 @@ export interface ExpressionWrapperProps { className?: string; canEdit: boolean; onRuntimeError: () => void; + onExpressionError: () => void; executionContext?: KibanaExecutionContext; lensInspector: LensInspector; noPadding?: boolean; @@ -53,9 +54,28 @@ export interface ExpressionWrapperProps { interface VisualizationErrorProps { errors: ExpressionWrapperProps['errors']; canEdit: boolean; + onExpressionError: ExpressionWrapperProps['onExpressionError']; + searchSessionId?: string; } -export function VisualizationErrorPanel({ errors, canEdit }: VisualizationErrorProps) { +export function VisualizationErrorPanel({ + errors, + canEdit, + onExpressionError, + searchSessionId, +}: VisualizationErrorProps) { + // use a combination of sessionid + error messages to decide whether to trigger a rerender + const rerenderKey = `${searchSessionId || ''}-${errors + ?.map(({ longMessage }) => longMessage) + .join('-')}`; + + const keyRef = useRef(rerenderKey); + // Skip error logging when no search session id is passed + if (keyRef.current !== rerenderKey) { + keyRef.current = rerenderKey; + onExpressionError(); + } + const showMore = errors && errors.length > 1; const canFixInLens = canEdit && errors?.some(({ type }) => type === 'fixable'); return ( @@ -121,6 +141,7 @@ export function ExpressionWrapper({ errors, canEdit, onRuntimeError, + onExpressionError, executionContext, lensInspector, noPadding, @@ -128,7 +149,12 @@ export function ExpressionWrapper({ return ( {errors || expression === null || expression === '' ? ( - + ) : (
Date: Wed, 26 Oct 2022 12:44:24 +0200 Subject: [PATCH 2/3] :wrench: Set complete also on error --- .../public/render_complete/render_complete_dispatcher.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts b/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts index f8fb9da67c901..597ddcaefc500 100644 --- a/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts +++ b/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts @@ -39,8 +39,9 @@ export class RenderCompleteDispatcher { public setEl(el?: HTMLElement) { if (this.el !== el) { - this.el = el; - this.count = 0; + this.el = el; + this.count = 0; + } if (el) this.dispatchInProgress(); } @@ -62,7 +63,7 @@ export class RenderCompleteDispatcher { public dispatchError() { if (!this.el) return; this.count++; - this.el.setAttribute('data-render-complete', 'false'); + this.el.setAttribute('data-render-complete', 'true'); this.el.setAttribute('data-rendering-count', String(this.count)); } From cad6f55e62e3182fa15bd3fb47aef4ab9a0f2d75 Mon Sep 17 00:00:00 2001 From: Uladzislau Lasitsa Date: Tue, 15 Nov 2022 15:29:08 +0200 Subject: [PATCH 3/3] Some updates --- .../render_complete_dispatcher.ts | 1 + .../lens/public/embeddable/embeddable.tsx | 16 ++++++---- .../public/embeddable/expression_wrapper.tsx | 32 ++----------------- 3 files changed, 13 insertions(+), 36 deletions(-) diff --git a/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts b/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts index 597ddcaefc500..8ae5cfbe0ca84 100644 --- a/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts +++ b/src/plugins/kibana_utils/public/render_complete/render_complete_dispatcher.ts @@ -64,6 +64,7 @@ export class RenderCompleteDispatcher { if (!this.el) return; this.count++; this.el.setAttribute('data-render-complete', 'true'); + this.el.setAttribute('data-loading', 'false'); this.el.setAttribute('data-rendering-count', String(this.count)); } diff --git a/x-pack/plugins/lens/public/embeddable/embeddable.tsx b/x-pack/plugins/lens/public/embeddable/embeddable.tsx index 6356bab529815..5bfc0c7608db0 100644 --- a/x-pack/plugins/lens/public/embeddable/embeddable.tsx +++ b/x-pack/plugins/lens/public/embeddable/embeddable.tsx @@ -651,10 +651,6 @@ export class Embeddable }); }; - private onError: ExpressionWrapperProps['onExpressionError'] = () => { - this.renderComplete.dispatchError(); - }; - getExecutionContext() { if (this.savedVis) { const parentContext = this.parent?.getInput().executionContext || this.input.executionContext; @@ -711,12 +707,19 @@ export class Embeddable this.domNode.setAttribute('data-shared-item', ''); + const error = this.getError(); + this.updateOutput({ ...this.getOutput(), loading: true, - error: this.getError(), + error, }); - this.renderComplete.dispatchInProgress(); + + if (error) { + this.renderComplete.dispatchError(); + } else { + this.renderComplete.dispatchInProgress(); + } const input = this.getInput(); @@ -746,7 +749,6 @@ export class Embeddable style={input.style} executionContext={this.getExecutionContext()} canEdit={this.getIsEditable() && input.viewMode === 'edit'} - onExpressionError={this.onError} onRuntimeError={(message) => { this.updateOutput({ error: new Error(message) }); this.logError('runtime'); diff --git a/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx b/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx index 0ec385523390b..cbb1fedf75497 100644 --- a/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx +++ b/x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useRef } from 'react'; +import React from 'react'; import { I18nProvider } from '@kbn/i18n-react'; import { FormattedMessage } from '@kbn/i18n-react'; import { EuiFlexGroup, EuiFlexItem, EuiText, EuiIcon, EuiEmptyPrompt } from '@elastic/eui'; @@ -45,7 +45,6 @@ export interface ExpressionWrapperProps { className?: string; canEdit: boolean; onRuntimeError: (message?: string) => void; - onExpressionError: () => void; executionContext?: KibanaExecutionContext; lensInspector: LensInspector; noPadding?: boolean; @@ -54,28 +53,9 @@ export interface ExpressionWrapperProps { interface VisualizationErrorProps { errors: ExpressionWrapperProps['errors']; canEdit: boolean; - onExpressionError: ExpressionWrapperProps['onExpressionError']; - searchSessionId?: string; } -export function VisualizationErrorPanel({ - errors, - canEdit, - onExpressionError, - searchSessionId, -}: VisualizationErrorProps) { - // use a combination of sessionid + error messages to decide whether to trigger a rerender - const rerenderKey = `${searchSessionId || ''}-${errors - ?.map(({ longMessage }) => longMessage) - .join('-')}`; - - const keyRef = useRef(rerenderKey); - // Skip error logging when no search session id is passed - if (keyRef.current !== rerenderKey) { - keyRef.current = rerenderKey; - onExpressionError(); - } - +export function VisualizationErrorPanel({ errors, canEdit }: VisualizationErrorProps) { const showMore = errors && errors.length > 1; const canFixInLens = canEdit && errors?.some(({ type }) => type === 'fixable'); return ( @@ -141,7 +121,6 @@ export function ExpressionWrapper({ errors, canEdit, onRuntimeError, - onExpressionError, executionContext, lensInspector, noPadding, @@ -149,12 +128,7 @@ export function ExpressionWrapper({ return ( {errors || expression === null || expression === '' ? ( - + ) : (