diff --git a/packages/grafana-ui/package.json b/packages/grafana-ui/package.json index 1653dd92b0f..5f9febd2444 100644 --- a/packages/grafana-ui/package.json +++ b/packages/grafana-ui/package.json @@ -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", diff --git a/packages/grafana-ui/src/components/GraphNG/GraphNG.test.tsx b/packages/grafana-ui/src/components/GraphNG/GraphNG.test.tsx deleted file mode 100644 index a53d5b02a16..00000000000 --- a/packages/grafana-ui/src/components/GraphNG/GraphNG.test.tsx +++ /dev/null @@ -1,167 +0,0 @@ -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, - }); - - 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( - // - // ); - // - // data.fields[1].values.set(0, 1); - // - // rerender( - // - // ); - // - // 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( - - ); - - 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( - // - // ); - // - // data.addField({ - // name: 'Value1', - // type: FieldType.number, - // values: new ArrayVector([1, 2, 3]), - // config: { - // custom: { - // line: { show: true }, - // }, - // } as FieldConfig, - // }); - // - // rerender( - // - // ); - // - // expect(onPlotInitSpy).toBeCalledTimes(2); - // }); - // - // it('reinitializes plot when series field config changes', () => { - // const { data, timeRange } = mockData(); - // const onPlotInitSpy = jest.fn(); - // - // const { rerender } = render( - // - // ); - // expect(onPlotInitSpy).toBeCalledTimes(1); - // - // data.fields[1].config.custom.line.width = 5; - // - // rerender( - // - // ); - // - // expect(onPlotInitSpy).toBeCalledTimes(2); - // }); - }); -}); diff --git a/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx b/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx index 4d4268d688f..8ae6f641c86 100755 --- a/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx +++ b/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx @@ -51,28 +51,27 @@ export const GraphNG: React.FC = ({ ...plotProps }) => { const alignedFrameWithGapTest = useMemo(() => alignDataFrames(data, fields), [data, fields]); - - if (alignedFrameWithGapTest == null) { - return ( -
-

No data found in response

-
- ); - } - const theme = useTheme(); const legendItemsRef = useRef([]); 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 = ({ return builder; }, [configRev]); + if (alignedFrameWithGapTest == null) { + return ( +
+

No data found in response

+
+ ); + } + let legendElement: React.ReactElement | undefined; if (hasLegend && legendItemsRef.current.length > 0) { diff --git a/packages/grafana-ui/src/components/uPlot/Plot.test.tsx b/packages/grafana-ui/src/components/uPlot/Plot.test.tsx new file mode 100644 index 00000000000..a514f310b15 --- /dev/null +++ b/packages/grafana-ui/src/components/uPlot/Plot.test.tsx @@ -0,0 +1,221 @@ +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, + }); + + 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( + + ); + + // 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( + + ); + + // 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( + + ); + + 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( + + ); + + 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( + + ); + + // we wait 1 frame for plugins initialisation logic to finish + act(() => { + mockRaf.step({ count: 1 }); + }); + + expect(uPlot).toBeCalledTimes(1); + + rerender( + + ); + + 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( + + ); + + // we wait 1 frame for plugins initialisation logic to finish + act(() => { + mockRaf.step({ count: 1 }); + }); + + rerender( + + ); + + expect(destroyMock).toBeCalledTimes(0); + expect(uPlot).toBeCalledTimes(1); + expect(setSizeMock).toBeCalledTimes(1); + }); + }); +}); diff --git a/packages/grafana-ui/src/components/uPlot/Plot.tsx b/packages/grafana-ui/src/components/uPlot/Plot.tsx index 40749114d02..8b202e18bd6 100755 --- a/packages/grafana-ui/src/components/uPlot/Plot.tsx +++ b/packages/grafana-ui/src/components/uPlot/Plot.tsx @@ -1,83 +1,78 @@ -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 = props => { const canvasRef = useRef(null); - const [plotInstance, setPlotInstance] = useState(); - const plotData = useRef(); - 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(); + 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]); + // 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; + } - useEffect(() => { - const currentInstance = plotInstance; - return () => { - currentInstance?.destroy(); - }; - }, [plotInstance]); - - // 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; } - }, [plotInstance, props.width, props.height]); + + // 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; + } + + // 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 = 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); } diff --git a/packages/grafana-ui/src/components/uPlot/hooks.ts b/packages/grafana-ui/src/components/uPlot/hooks.ts index 5a46c8f7e70..1cb0e07dd1d 100644 --- a/packages/grafana-ui/src/components/uPlot/hooks.ts +++ b/packages/grafana-ui/src/components/uPlot/hooks.ts @@ -1,4 +1,4 @@ -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(); + const [isConfigReady, setIsConfigReady] = useState(false); + const currentConfig = useRef(); 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(dep: T, cmp: (prev: T, next: T) => boolean) { +export function useRevision(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]); diff --git a/yarn.lock b/yarn.lock index 3c960c18d6d..451a4d583fd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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"