Skip to content

Commit

Permalink
[Lens] Make sure Lens does not reload unnecessarily (#86092)
Browse files Browse the repository at this point in the history
  • Loading branch information
flash1293 authored Dec 21, 2020
1 parent 6d72042 commit 388b250
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 42 deletions.
38 changes: 38 additions & 0 deletions src/plugins/expressions/public/react_expression_renderer.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,44 @@ describe('ExpressionRenderer', () => {
instance.unmount();
});

it('waits for debounce period on other loader option change if specified', () => {
jest.useFakeTimers();

const refreshSubject = new Subject();
const loaderUpdate = jest.fn();

(ExpressionLoader as jest.Mock).mockImplementation(() => {
return {
render$: new Subject(),
data$: new Subject(),
loading$: new Subject(),
update: loaderUpdate,
destroy: jest.fn(),
};
});

const instance = mount(
<ReactExpressionRenderer
reload$={refreshSubject}
expression=""
debounce={1000}
searchContext={{ from: 'now-15m', to: 'now' }}
/>
);

instance.setProps({ searchContext: { from: 'now-30m', to: 'now' } });

expect(loaderUpdate).toHaveBeenCalledTimes(1);

act(() => {
jest.runAllTimers();
});

expect(loaderUpdate).toHaveBeenCalledTimes(2);

instance.unmount();
});

it('should display a custom error message if the user provides one and then remove it after successful render', () => {
const dataSubject = new Subject();
const data$ = dataSubject.asObservable().pipe(share());
Expand Down
18 changes: 12 additions & 6 deletions src/plugins/expressions/public/react_expression_renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,23 @@ export const ReactExpressionRenderer = ({
null
);
const [debouncedExpression, setDebouncedExpression] = useState(expression);
useEffect(() => {
const [waitingForDebounceToComplete, setDebouncePending] = useState(false);
useShallowCompareEffect(() => {
if (debounce === undefined) {
return;
}
setDebouncePending(true);
const handler = setTimeout(() => {
setDebouncedExpression(expression);
setDebouncePending(false);
}, debounce);

return () => {
clearTimeout(handler);
};
}, [expression, debounce]);
}, [expression, expressionLoaderOptions, debounce]);

const activeExpression = debounce !== undefined ? debouncedExpression : expression;
const waitingForDebounceToComplete = debounce !== undefined && expression !== debouncedExpression;

/* eslint-disable react-hooks/exhaustive-deps */
// OK to ignore react-hooks/exhaustive-deps because options update is handled by calling .update()
Expand Down Expand Up @@ -182,12 +184,16 @@ export const ReactExpressionRenderer = ({
// Re-fetch data automatically when the inputs change
useShallowCompareEffect(
() => {
if (expressionLoaderRef.current) {
// only update the loader if the debounce period is over
if (expressionLoaderRef.current && !waitingForDebounceToComplete) {
expressionLoaderRef.current.update(activeExpression, expressionLoaderOptions);
}
},
// when expression is changed by reference and when any other loaderOption is changed by reference
[{ activeExpression, ...expressionLoaderOptions }]
// when debounced, wait for debounce status to change to update loader.
// Otherwise, update when expression is changed by reference and when any other loaderOption is changed by reference
debounce === undefined
? [{ activeExpression, ...expressionLoaderOptions }]
: [{ waitingForDebounceToComplete }]
);

/* eslint-enable react-hooks/exhaustive-deps */
Expand Down
80 changes: 48 additions & 32 deletions x-pack/plugins/lens/public/app_plugin/mounter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import React from 'react';
import React, { useCallback } from 'react';

import { AppMountParameters, CoreSetup } from 'kibana/public';
import { FormattedMessage, I18nProvider } from '@kbn/i18n/react';
import { HashRouter, Route, RouteComponentProps, Switch } from 'react-router-dom';
import { History } from 'history';
import { render, unmountComponentAtNode } from 'react-dom';
import { i18n } from '@kbn/i18n';

Expand Down Expand Up @@ -86,25 +87,22 @@ export async function mountApp(
})
);

const getInitialInput = (
routeProps: RouteComponentProps<{ id?: string }>,
editByValue?: boolean
): LensEmbeddableInput | undefined => {
const getInitialInput = (id?: string, editByValue?: boolean): LensEmbeddableInput | undefined => {
if (editByValue) {
return embeddableEditorIncomingState?.valueInput as LensByValueInput;
}
if (routeProps.match.params.id) {
return { savedObjectId: routeProps.match.params.id } as LensByReferenceInput;
if (id) {
return { savedObjectId: id } as LensByReferenceInput;
}
};

const redirectTo = (routeProps: RouteComponentProps<{ id?: string }>, savedObjectId?: string) => {
const redirectTo = (history: History<unknown>, savedObjectId?: string) => {
if (!savedObjectId) {
routeProps.history.push({ pathname: '/', search: routeProps.history.location.search });
history.push({ pathname: '/', search: history.location.search });
} else {
routeProps.history.push({
history.push({
pathname: `/edit/${savedObjectId}`,
search: routeProps.history.location.search,
search: history.location.search,
});
}
};
Expand Down Expand Up @@ -144,27 +142,45 @@ export async function mountApp(
}
};

const renderEditor = (
routeProps: RouteComponentProps<{ id?: string }>,
editByValue?: boolean
// const featureFlagConfig = await getByValueFeatureFlag();
const EditorRenderer = React.memo(
(props: { id?: string; history: History<unknown>; editByValue?: boolean }) => {
const redirectCallback = useCallback(
(id?: string) => {
redirectTo(props.history, id);
},
[props.history]
);
trackUiEvent('loaded');
return (
<App
incomingState={embeddableEditorIncomingState}
editorFrame={instance}
initialInput={getInitialInput(props.id, props.editByValue)}
redirectTo={redirectCallback}
redirectToOrigin={redirectToOrigin}
redirectToDashboard={redirectToDashboard}
onAppLeave={params.onAppLeave}
setHeaderActionMenu={params.setHeaderActionMenu}
history={props.history}
initialContext={
historyLocationState && historyLocationState.type === ACTION_VISUALIZE_LENS_FIELD
? historyLocationState.payload
: undefined
}
/>
);
}
);

const EditorRoute = (
routeProps: RouteComponentProps<{ id?: string }> & { editByValue?: boolean }
) => {
trackUiEvent('loaded');
return (
<App
incomingState={embeddableEditorIncomingState}
editorFrame={instance}
initialInput={getInitialInput(routeProps, editByValue)}
redirectTo={(savedObjectId?: string) => redirectTo(routeProps, savedObjectId)}
redirectToOrigin={redirectToOrigin}
redirectToDashboard={redirectToDashboard}
onAppLeave={params.onAppLeave}
setHeaderActionMenu={params.setHeaderActionMenu}
<EditorRenderer
id={routeProps.match.params.id}
history={routeProps.history}
initialContext={
historyLocationState && historyLocationState.type === ACTION_VISUALIZE_LENS_FIELD
? historyLocationState.payload
: undefined
}
editByValue={routeProps.editByValue}
/>
);
};
Expand All @@ -185,13 +201,13 @@ export async function mountApp(
<KibanaContextProvider services={lensServices}>
<HashRouter>
<Switch>
<Route exact path="/edit/:id" render={renderEditor} />
<Route exact path="/edit/:id" component={EditorRoute} />
<Route
exact
path={`/${LENS_EDIT_BY_VALUE}`}
render={(routeProps) => renderEditor(routeProps, true)}
render={(routeProps) => <EditorRoute {...routeProps} editByValue />}
/>
<Route exact path="/" render={renderEditor} />
<Route exact path="/" component={EditorRoute} />
<Route path="/" component={NotFound} />
</Switch>
</HashRouter>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import './suggestion_panel.scss';

import _, { camelCase } from 'lodash';
import React, { useState, useEffect, useMemo } from 'react';
import React, { useState, useEffect, useMemo, useRef } from 'react';
import { FormattedMessage } from '@kbn/i18n/react';
import {
EuiIcon,
Expand Down Expand Up @@ -270,13 +270,19 @@ export function SuggestionPanel({
[frame.query, frame.dateRange.fromDate, frame.dateRange.toDate, frame.filters]
);

const contextRef = useRef<ExecutionContextSearch>(context);
contextRef.current = context;

const AutoRefreshExpressionRenderer = useMemo(() => {
const autoRefreshFetch$ = plugins.data.query.timefilter.timefilter.getAutoRefreshFetch$();
return (props: ReactExpressionRendererProps) => (
<ExpressionRendererComponent {...props} searchContext={context} reload$={autoRefreshFetch$} />
<ExpressionRendererComponent
{...props}
searchContext={contextRef.current}
reload$={autoRefreshFetch$}
/>
);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [plugins.data.query.timefilter.timefilter, context]);
}, [plugins.data.query.timefilter.timefilter, ExpressionRendererComponent]);

const [lastSelectedSuggestion, setLastSelectedSuggestion] = useState<number>(-1);

Expand Down

0 comments on commit 388b250

Please sign in to comment.