Commit 1ac02390 by Dominik Prokop Committed by GitHub

GraphNG: simplify effects responsible for plot updates/initialization (#29496)

* Add test coverage for uPlot initialization and data updates

* fix ts

* WIP

* I see some light in the tunel

* Tests update

* Fix rendering before hooks are evaluated
parent d33c4e5e
......@@ -89,6 +89,7 @@
"@types/jest": "26.0.15",
"@types/jquery": "3.3.38",
"@types/lodash": "4.14.123",
"@types/mock-raf": "1.0.2",
"@types/node": "10.14.1",
"@types/papaparse": "5.2.0",
"@types/react": "16.9.9",
......@@ -98,6 +99,7 @@
"@types/rollup-plugin-visualizer": "2.6.0",
"@types/tinycolor2": "1.4.1",
"common-tags": "^1.8.0",
"mock-raf": "1.0.1",
"pretty-format": "25.1.0",
"react-docgen-typescript-loader": "3.7.2",
"react-test-renderer": "16.13.1",
......
import React from 'react';
import { GraphNG } from './GraphNG';
import { render } from '@testing-library/react';
import { ArrayVector, dateTime, FieldConfig, FieldType, MutableDataFrame } from '@grafana/data';
import { GraphFieldConfig, GraphMode } from '../uPlot/config';
const mockData = () => {
const data = new MutableDataFrame();
data.addField({
type: FieldType.time,
name: 'Time',
values: new ArrayVector([1602630000000, 1602633600000, 1602637200000]),
config: {},
});
data.addField({
type: FieldType.number,
name: 'Value',
values: new ArrayVector([10, 20, 5]),
config: {
custom: {
mode: GraphMode.Line,
},
} as FieldConfig<GraphFieldConfig>,
});
const timeRange = {
from: dateTime(1602673200000),
to: dateTime(1602680400000),
raw: { from: '1602673200000', to: '1602680400000' },
};
return { data, timeRange };
};
// const defaultLegendOptions: LegendOptions = {
// displayMode: LegendDisplayMode.List,
// placement: 'bottom',
// };
describe('GraphNG', () => {
// describe('data update', () => {
// it('does not re-initialise uPlot when there are no field config changes', () => {
// const { data, timeRange } = mockData();
// const onDataUpdateSpy = jest.fn();
// const onPlotInitSpy = jest.fn();
//
// const { rerender } = render(
// <GraphNG
// data={[data]}
// timeRange={timeRange}
// timeZone={'browser'}
// width={100}
// height={100}
// onDataUpdate={onDataUpdateSpy}
// onPlotInit={onPlotInitSpy}
// legend={defaultLegendOptions}
// />
// );
//
// data.fields[1].values.set(0, 1);
//
// rerender(
// <GraphNG
// data={[data]}
// timeRange={timeRange}
// timeZone={'browser'}
// width={100}
// height={100}
// onDataUpdate={onDataUpdateSpy}
// onPlotInit={onPlotInitSpy}
// legend={defaultLegendOptions}
// />
// );
//
// expect(onPlotInitSpy).toBeCalledTimes(1);
// expect(onDataUpdateSpy).toHaveBeenLastCalledWith([
// [1602630000, 1602633600, 1602637200],
// [1, 20, 5],
// ]);
// });
// });
describe('config update', () => {
it('should skip plot intialization for width and height equal 0', () => {
const { data, timeRange } = mockData();
const { queryAllByTestId } = render(
<GraphNG data={[data]} timeRange={timeRange} timeZone={'browser'} width={0} height={0} />
);
expect(queryAllByTestId('uplot-main-div')).toHaveLength(1);
});
// it('reinitializes plot when number of series change', () => {
// const { data, timeRange } = mockData();
// const onPlotInitSpy = jest.fn();
//
// const { rerender } = render(
// <GraphNG
// data={[data]}
// timeRange={timeRange}
// timeZone={'browser'}
// width={100}
// height={100}
// onPlotInit={onPlotInitSpy}
// />
// );
//
// data.addField({
// name: 'Value1',
// type: FieldType.number,
// values: new ArrayVector([1, 2, 3]),
// config: {
// custom: {
// line: { show: true },
// },
// } as FieldConfig<GraphCustomFieldConfig>,
// });
//
// rerender(
// <GraphNG
// data={[data]}
// timeRange={timeRange}
// timeZone={'browser'}
// width={100}
// height={100}
// onPlotInit={onPlotInitSpy}
// />
// );
//
// expect(onPlotInitSpy).toBeCalledTimes(2);
// });
//
// it('reinitializes plot when series field config changes', () => {
// const { data, timeRange } = mockData();
// const onPlotInitSpy = jest.fn();
//
// const { rerender } = render(
// <GraphNG
// data={[data]}
// timeRange={timeRange}
// timeZone={'browser'}
// width={100}
// height={100}
// onPlotInit={onPlotInitSpy}
// />
// );
// expect(onPlotInitSpy).toBeCalledTimes(1);
//
// data.fields[1].config.custom.line.width = 5;
//
// rerender(
// <GraphNG
// data={[data]}
// timeRange={timeRange}
// timeZone={'browser'}
// width={100}
// height={100}
// onPlotInit={onPlotInitSpy}
// />
// );
//
// expect(onPlotInitSpy).toBeCalledTimes(2);
// });
});
});
......@@ -51,28 +51,27 @@ export const GraphNG: React.FC<GraphNGProps> = ({
...plotProps
}) => {
const alignedFrameWithGapTest = useMemo(() => alignDataFrames(data, fields), [data, fields]);
if (alignedFrameWithGapTest == null) {
return (
<div className="panel-empty">
<p>No data found in response</p>
</div>
);
}
const theme = useTheme();
const legendItemsRef = useRef<LegendItem[]>([]);
const hasLegend = useRef(legend && legend.displayMode !== LegendDisplayMode.Hidden);
const alignedFrame = alignedFrameWithGapTest.frame;
const compareFrames = useCallback(
(a: DataFrame, b: DataFrame) => compareDataFrameStructures(a, b, ['min', 'max']),
[]
);
const alignedFrame = alignedFrameWithGapTest?.frame;
const compareFrames = useCallback((a?: DataFrame | null, b?: DataFrame | null) => {
if (a && b) {
return compareDataFrameStructures(a, b, ['min', 'max']);
}
return false;
}, []);
const configRev = useRevision(alignedFrame, compareFrames);
const configBuilder = useMemo(() => {
const builder = new UPlotConfigBuilder();
if (!alignedFrame) {
return builder;
}
// X is the first field in the alligned frame
const xField = alignedFrame.fields[0];
if (xField.type === FieldType.time) {
......@@ -163,6 +162,14 @@ export const GraphNG: React.FC<GraphNGProps> = ({
return builder;
}, [configRev]);
if (alignedFrameWithGapTest == null) {
return (
<div className="panel-empty">
<p>No data found in response</p>
</div>
);
}
let legendElement: React.ReactElement | undefined;
if (hasLegend && legendItemsRef.current.length > 0) {
......
import React from 'react';
import { UPlotChart } from './Plot';
import { act, render } from '@testing-library/react';
import { ArrayVector, dateTime, FieldConfig, FieldType, MutableDataFrame } from '@grafana/data';
import { GraphFieldConfig, GraphMode } from '../uPlot/config';
import uPlot from 'uplot';
import createMockRaf from 'mock-raf';
import { UPlotConfigBuilder } from './config/UPlotConfigBuilder';
const mockRaf = createMockRaf();
const setDataMock = jest.fn();
const setSizeMock = jest.fn();
const initializeMock = jest.fn();
const destroyMock = jest.fn();
jest.mock('uplot', () => {
return jest.fn().mockImplementation(() => {
return {
setData: setDataMock,
setSize: setSizeMock,
initialize: initializeMock,
destroy: destroyMock,
};
});
});
const mockData = () => {
const data = new MutableDataFrame();
data.addField({
type: FieldType.time,
name: 'Time',
values: new ArrayVector([1602630000000, 1602633600000, 1602637200000]),
config: {},
});
data.addField({
type: FieldType.number,
name: 'Value',
values: new ArrayVector([10, 20, 5]),
config: {
custom: {
mode: GraphMode.Line,
},
} as FieldConfig<GraphFieldConfig>,
});
const timeRange = {
from: dateTime(1602673200000),
to: dateTime(1602680400000),
raw: { from: '1602673200000', to: '1602680400000' },
};
return { data, timeRange, config: new UPlotConfigBuilder() };
};
describe('UPlotChart', () => {
beforeEach(() => {
// @ts-ignore
uPlot.mockClear();
setDataMock.mockClear();
setSizeMock.mockClear();
initializeMock.mockClear();
destroyMock.mockClear();
jest.spyOn(window, 'requestAnimationFrame').mockImplementation(mockRaf.raf);
});
it('destroys uPlot instance when component unmounts', () => {
const { data, timeRange, config } = mockData();
const uPlotData = { frame: data, isGap: () => false };
const { unmount } = render(
<UPlotChart
data={uPlotData}
config={config}
timeRange={timeRange}
timeZone={'browser'}
width={100}
height={100}
/>
);
// we wait 1 frame for plugins initialisation logic to finish
act(() => {
mockRaf.step({ count: 1 });
});
expect(uPlot).toBeCalledTimes(1);
unmount();
expect(destroyMock).toBeCalledTimes(1);
});
describe('data update', () => {
it('skips uPlot reinitialization when there are no field config changes', () => {
const { data, timeRange, config } = mockData();
const uPlotData = { frame: data, isGap: () => false };
const { rerender } = render(
<UPlotChart
data={uPlotData}
config={config}
timeRange={timeRange}
timeZone={'browser'}
width={100}
height={100}
/>
);
// we wait 1 frame for plugins initialisation logic to finish
act(() => {
mockRaf.step({ count: 1 });
});
expect(uPlot).toBeCalledTimes(1);
data.fields[1].values.set(0, 1);
uPlotData.frame = data;
rerender(
<UPlotChart
data={uPlotData}
config={config}
timeRange={timeRange}
timeZone={'browser'}
width={100}
height={100}
/>
);
expect(setDataMock).toBeCalledTimes(1);
});
});
describe('config update', () => {
it('skips uPlot intialization for width and height equal 0', async () => {
const { data, timeRange, config } = mockData();
const uPlotData = { frame: data, isGap: () => false };
const { queryAllByTestId } = render(
<UPlotChart data={uPlotData} config={config} timeRange={timeRange} timeZone={'browser'} width={0} height={0} />
);
expect(queryAllByTestId('uplot-main-div')).toHaveLength(1);
expect(uPlot).not.toBeCalled();
});
it('reinitializes uPlot when config changes', () => {
const { data, timeRange, config } = mockData();
const uPlotData = { frame: data, isGap: () => false };
const { rerender } = render(
<UPlotChart
data={uPlotData}
config={config}
timeRange={timeRange}
timeZone={'browser'}
width={100}
height={100}
/>
);
// we wait 1 frame for plugins initialisation logic to finish
act(() => {
mockRaf.step({ count: 1 });
});
expect(uPlot).toBeCalledTimes(1);
rerender(
<UPlotChart
data={uPlotData}
config={new UPlotConfigBuilder()}
timeRange={timeRange}
timeZone={'browser'}
width={100}
height={100}
/>
);
expect(destroyMock).toBeCalledTimes(1);
expect(uPlot).toBeCalledTimes(2);
});
it('skips uPlot reinitialization when only dimensions change', () => {
const { data, timeRange, config } = mockData();
const uPlotData = { frame: data, isGap: () => false };
const { rerender } = render(
<UPlotChart
data={uPlotData}
config={config}
timeRange={timeRange}
timeZone={'browser'}
width={100}
height={100}
/>
);
// we wait 1 frame for plugins initialisation logic to finish
act(() => {
mockRaf.step({ count: 1 });
});
rerender(
<UPlotChart
data={uPlotData}
config={new UPlotConfigBuilder()}
timeRange={timeRange}
timeZone={'browser'}
width={200}
height={200}
/>
);
expect(destroyMock).toBeCalledTimes(0);
expect(uPlot).toBeCalledTimes(1);
expect(setSizeMock).toBeCalledTimes(1);
});
});
});
import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react';
import uPlot, { AlignedData, AlignedDataWithGapTest } from 'uplot';
import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef } from 'react';
import uPlot, { AlignedData, AlignedDataWithGapTest, Options } from 'uplot';
import { buildPlotContext, PlotContext } from './context';
import { pluginLog } from './utils';
import { usePlotConfig } from './hooks';
import { PlotProps } from './types';
import { usePrevious } from 'react-use';
import { AlignedFrameWithGapTest, PlotProps } from './types';
import { DataFrame, FieldType } from '@grafana/data';
import isNumber from 'lodash/isNumber';
import { UPlotConfigBuilder } from './config/UPlotConfigBuilder';
import usePrevious from 'react-use/lib/usePrevious';
// uPlot abstraction responsible for plot initialisation, setup and refresh
// Receives a data frame that is x-axis aligned, as of https://github.com/leeoniya/uPlot/tree/master/docs#data-format
// Exposes contexts for plugins registration and uPlot instance access
export const UPlotChart: React.FC<PlotProps> = props => {
const canvasRef = useRef<HTMLDivElement>(null);
const [plotInstance, setPlotInstance] = useState<uPlot>();
const plotData = useRef<AlignedDataWithGapTest>();
const previousConfig = usePrevious(props.config);
// uPlot config API
const { currentConfig, registerPlugin } = usePlotConfig(props.width, props.height, props.timeZone, props.config);
const initializePlot = useCallback(() => {
if (!currentConfig || !plotData) {
return;
}
if (!canvasRef.current) {
throw new Error('Missing Canvas component as a child of the plot.');
}
pluginLog('UPlotChart: init uPlot', false, 'initialized with', plotData.current, currentConfig);
const instance = new uPlot(currentConfig, plotData.current, canvasRef.current);
setPlotInstance(instance);
}, [setPlotInstance, currentConfig]);
const plotInstance = useRef<uPlot>();
const prevProps = usePrevious(props);
const { isConfigReady, currentConfig, registerPlugin } = usePlotConfig(
props.width,
props.height,
props.timeZone,
props.config
);
const getPlotInstance = useCallback(() => {
if (!plotInstance) {
if (!plotInstance.current) {
throw new Error("Plot hasn't initialised yet");
}
return plotInstance;
}, [plotInstance]);
return plotInstance.current;
}, []);
// Effect responsible for uPlot updates/initialization logic. It's performed whenever component's props have changed
useLayoutEffect(() => {
plotData.current = {
data: props.data.frame.fields.map(f => f.values.toArray()) as AlignedData,
isGap: props.data.isGap,
};
if (plotInstance && previousConfig === props.config) {
updateData(props.data.frame, props.config, plotInstance, plotData.current.data);
// 0. Exit early if the component is not ready to initialize uPlot
if (!currentConfig.current || !canvasRef.current || props.width === 0 || props.height === 0) {
return;
}
}, [props.data, props.config]);
useLayoutEffect(() => {
initializePlot();
}, [currentConfig]);
useEffect(() => {
const currentInstance = plotInstance;
return () => {
currentInstance?.destroy();
};
}, [plotInstance]);
// 1. When config is ready and there is no uPlot instance, create new uPlot and return
if (isConfigReady && !plotInstance.current) {
plotInstance.current = initializePlot(prepareData(props.data), currentConfig.current, canvasRef.current);
return;
}
// When size props changed update plot size synchronously
useLayoutEffect(() => {
if (plotInstance) {
plotInstance.setSize({
width: props.width,
height: props.height,
// 2. When dimensions have changed, update uPlot size and return
if (currentConfig.current.width !== prevProps?.width || currentConfig.current.height !== prevProps?.height) {
pluginLog('uPlot core', false, 'updating size');
plotInstance.current!.setSize({
width: currentConfig.current.width,
height: currentConfig.current?.height,
});
return;
}
// 3. When config or timezone has changed, re-initialize plot
if (isConfigReady && (props.config !== prevProps.config || props.timeZone !== prevProps.timeZone)) {
if (plotInstance.current) {
pluginLog('uPlot core', false, 'destroying instance');
plotInstance.current.destroy();
}
plotInstance.current = initializePlot(prepareData(props.data), currentConfig.current, canvasRef.current);
return;
}
}, [plotInstance, props.width, props.height]);
// 4. Otherwise, assume only data has changed and update uPlot data
updateData(props.data.frame, props.config, plotInstance.current, prepareData(props.data).data);
}, [props, isConfigReady]);
// When component unmounts, clean the existing uPlot instance
useEffect(() => () => plotInstance.current?.destroy(), []);
// Memoize plot context
const plotCtx = useMemo(() => {
return buildPlotContext(Boolean(plotInstance), canvasRef, props.data, registerPlugin, getPlotInstance);
return buildPlotContext(Boolean(plotInstance.current), canvasRef, props.data, registerPlugin, getPlotInstance);
}, [plotInstance, canvasRef, props.data, registerPlugin, getPlotInstance]);
return (
......@@ -88,14 +83,24 @@ export const UPlotChart: React.FC<PlotProps> = props => {
);
};
// Callback executed when there was no change in plot config
function prepareData(data: AlignedFrameWithGapTest) {
return {
data: data.frame.fields.map(f => f.values.toArray()) as AlignedData,
isGap: data.isGap,
};
}
function initializePlot(data: AlignedDataWithGapTest, config: Options, el: HTMLDivElement) {
pluginLog('UPlotChart: init uPlot', false, 'initialized with', data, config);
return new uPlot(config, data, el);
}
function updateData(frame: DataFrame, config: UPlotConfigBuilder, plotInstance?: uPlot, data?: AlignedData | null) {
if (!plotInstance || !data) {
return;
}
pluginLog('uPlot core', false, 'updating plot data(throttled log!)', data);
updateScales(frame, config, plotInstance);
// If config hasn't changed just update uPlot's data
plotInstance.setData(data);
}
......
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react';
import { PlotPlugin } from './types';
import { pluginLog } from './utils';
import uPlot, { Options } from 'uplot';
......@@ -108,8 +108,9 @@ export const DEFAULT_PLOT_CONFIG = {
//pass plain confsig object,memoize!
export const usePlotConfig = (width: number, height: number, timeZone: TimeZone, configBuilder: UPlotConfigBuilder) => {
const { arePluginsReady, plugins, registerPlugin } = usePlotPlugins();
const [currentConfig, setCurrentConfig] = useState<Options>();
const [isConfigReady, setIsConfigReady] = useState(false);
const currentConfig = useRef<Options>();
const tzDate = useMemo(() => {
let fmt = undefined;
......@@ -122,11 +123,11 @@ export const usePlotConfig = (width: number, height: number, timeZone: TimeZone,
return fmt;
}, [timeZone]);
useEffect(() => {
useLayoutEffect(() => {
if (!arePluginsReady) {
return;
}
setCurrentConfig({
currentConfig.current = {
...DEFAULT_PLOT_CONFIG,
width,
height,
......@@ -136,10 +137,13 @@ export const usePlotConfig = (width: number, height: number, timeZone: TimeZone,
})),
tzDate,
...configBuilder.getConfig(),
});
};
setIsConfigReady(true);
}, [arePluginsReady, plugins, width, height, tzDate, configBuilder]);
return {
isConfigReady,
registerPlugin,
currentConfig,
};
......@@ -174,13 +178,14 @@ export const useRefreshAfterGraphRendered = (pluginId: string) => {
return renderToken;
};
export function useRevision<T>(dep: T, cmp: (prev: T, next: T) => boolean) {
export function useRevision<T>(dep?: T | null, cmp?: (prev?: T | null, next?: T | null) => boolean) {
const [rev, setRev] = useState(0);
const prevDep = usePrevious(dep);
const comparator = cmp ? cmp : (a?: T | null, b?: T | null) => a === b;
useEffect(() => {
const hasConfigChanged = prevDep ? !cmp(prevDep, dep) : true;
if (hasConfigChanged) {
useLayoutEffect(() => {
const hasChange = !comparator(prevDep, dep);
if (hasChange) {
setRev(r => r + 1);
}
}, [dep]);
......
......@@ -6648,6 +6648,11 @@
resolved "https://registry.yarnpkg.com/@types/minimatch/-/minimatch-3.0.3.tgz#3dca0e3f33b200fc7d1139c0cd96c1268cadfd9d"
integrity sha512-tHq6qdbT9U1IRSGf14CL0pUlULksvY9OZ+5eEgl1N7t+OA3tGvNpxJCzuKQlsNgCVwbAs670L1vcVQi8j9HjnA==
"@types/mock-raf@1.0.2":
version "1.0.2"
resolved "https://registry.yarnpkg.com/@types/mock-raf/-/mock-raf-1.0.2.tgz#de0df16b1cbe2475cb1a4680a19f344f386d8252"
integrity sha1-3g3xaxy+JHXLGkaAoZ80TzhtglI=
"@types/moment-timezone@0.5.13":
version "0.5.13"
resolved "https://registry.yarnpkg.com/@types/moment-timezone/-/moment-timezone-0.5.13.tgz#0317ccc91eb4c7f4901704166166395c39276528"
......@@ -18484,6 +18489,13 @@ mocha@7.0.1:
yargs-parser "13.1.1"
yargs-unparser "1.6.0"
mock-raf@1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/mock-raf/-/mock-raf-1.0.1.tgz#7567d23fa1220439853317a8ff0eaad1588e9535"
integrity sha512-+25y56bblLzEnv+G4ODsHNck07A5uP5HFfu/1VBKeFrUXoFT9oru+R+jLxLz6rwdM5drUHFdqX9LYBsMP4dz/w==
dependencies:
object-assign "^3.0.0"
modify-values@^1.0.0:
version "1.0.1"
resolved "https://registry.yarnpkg.com/modify-values/-/modify-values-1.0.1.tgz#b3939fa605546474e3e3e3c63d64bd43b4ee6022"
......@@ -19138,6 +19150,11 @@ object-assign@4.x, object-assign@^4.0.1, object-assign@^4.1.0, object-assign@^4.
resolved "https://registry.yarnpkg.com/object-assign/-/object-assign-4.1.1.tgz#2109adc7965887cfc05cbbd442cac8bfbb360863"
integrity sha1-IQmtx5ZYh8/AXLvUQsrIv7s2CGM=
object-assign@^3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/object-assign/-/object-assign-3.0.0.tgz#9bedd5ca0897949bca47e7ff408062d549f587f2"
integrity sha1-m+3VygiXlJvKR+f/QIBi1Un1h/I=
object-copy@^0.1.0:
version "0.1.0"
resolved "https://registry.yarnpkg.com/object-copy/-/object-copy-0.1.0.tgz#7e7d858b781bd7c991a41ba975ed3812754e998c"
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment