Skip to content

Commit

Permalink
Explore: Fix graph not updating when changing config (#62473)
Browse files Browse the repository at this point in the history
* Explore: Fix graph not updating when changing config

* move useStructureRev to a seprate hook and add specific tests
  • Loading branch information
Elfo404 authored Feb 1, 2023
1 parent 13c4ee1 commit 6b6b733
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 22 deletions.
38 changes: 16 additions & 22 deletions public/app/features/explore/Graph/ExploreGraph.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { css, cx } from '@emotion/css';
import { identity } from 'lodash';
import React, { useEffect, useMemo, useState } from 'react';
import { useCounter } from 'react-use';

import {
AbsoluteTimeRange,
Expand Down Expand Up @@ -37,6 +36,7 @@ import { ExploreGraphStyle } from 'app/types';
import { seriesVisibilityConfigFactory } from '../../dashboard/dashgrid/SeriesVisibilityConfigFactory';

import { applyGraphStyle } from './exploreGraphStyleUtils';
import { useStructureRev } from './useStructureRev';

const MAX_NUMBER_OF_TIME_SERIES = 20;

Expand Down Expand Up @@ -76,7 +76,15 @@ export function ExploreGraph({
const theme = useTheme2();
const style = useStyles2(getStyles);
const [showAllTimeSeries, setShowAllTimeSeries] = useState(false);
const [structureRev, { inc }] = useCounter(0);

const timeRange = {
from: dateTime(absoluteRange.from),
to: dateTime(absoluteRange.to),
raw: {
from: dateTime(absoluteRange.from),
to: dateTime(absoluteRange.to),
},
};

const fieldConfigRegistry = useMemo(
() => createFieldConfigRegistry(getGraphFieldConfig(defaultGraphConfig), 'Explore'),
Expand All @@ -98,34 +106,20 @@ export function ExploreGraph({
overrides: [],
});

const timeRange = {
from: dateTime(absoluteRange.from),
to: dateTime(absoluteRange.to),
raw: {
from: dateTime(absoluteRange.from),
to: dateTime(absoluteRange.to),
},
};

const styledFieldConfig = useMemo(() => applyGraphStyle(fieldConfig, graphStyle), [fieldConfig, graphStyle]);

const dataWithConfig = useMemo(() => {
return applyFieldOverrides({
fieldConfig: styledFieldConfig,
data,
data: showAllTimeSeries ? data : data.slice(0, MAX_NUMBER_OF_TIME_SERIES),
timeZone,
replaceVariables: (value) => value, // We don't need proper replace here as it is only used in getLinks and we use getFieldLinks
theme,
fieldConfigRegistry,
});
}, [fieldConfigRegistry, data, timeZone, theme, styledFieldConfig]);

const seriesToShow = showAllTimeSeries ? dataWithConfig : dataWithConfig.slice(0, MAX_NUMBER_OF_TIME_SERIES);
}, [fieldConfigRegistry, data, timeZone, theme, styledFieldConfig, showAllTimeSeries]);

// We need to increment structureRev when the number of series changes.
// the function passed to useMemo runs during rendering, so when we get a different
// amount of data, structureRev is incremented before we render it
useMemo(inc, [dataWithConfig.length, styledFieldConfig, seriesToShow.length, inc]);
const structureRev = useStructureRev(dataWithConfig);

useEffect(() => {
if (onHiddenSeriesChanged) {
Expand Down Expand Up @@ -164,7 +158,7 @@ export function ExploreGraph({

return (
<PanelContextProvider value={panelContext}>
{dataWithConfig.length > MAX_NUMBER_OF_TIME_SERIES && !showAllTimeSeries && (
{data.length > MAX_NUMBER_OF_TIME_SERIES && !showAllTimeSeries && (
<div className={cx([style.timeSeriesDisclaimer])}>
<Icon className={style.disclaimerIcon} name="exclamation-triangle" />
Showing only {MAX_NUMBER_OF_TIME_SERIES} time series.
Expand All @@ -174,12 +168,12 @@ export function ExploreGraph({
onClick={() => setShowAllTimeSeries(true)}
className={style.showAllButton}
>
Show all {dataWithConfig.length}
Show all {data.length}
</Button>
</div>
)}
<PanelRenderer
data={{ series: seriesToShow, timeRange, state: loadingState, annotations, structureRev }}
data={{ series: dataWithConfig, timeRange, state: loadingState, annotations, structureRev }}
pluginId="timeseries"
title=""
width={width}
Expand Down
117 changes: 117 additions & 0 deletions public/app/features/explore/Graph/useStructureRev.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { renderHook } from '@testing-library/react-hooks';

import { DataFrame, FieldType, toDataFrame } from '@grafana/data';

import { useStructureRev } from './useStructureRev';

let lastResult: number = Number.MIN_SAFE_INTEGER;

const resetCounters = () => {
lastResult = Number.MIN_SAFE_INTEGER;
};

const startCounters = (start: number | Error) => {
// The if is only to make TypeScript happy
if (start instanceof Error) {
expect(start).not.toBeInstanceOf(Error);
return;
}

lastResult = start;
};

beforeAll(() => {
expect.extend({
toHaveIncremented(received: number[]) {
if (received.length < 2) {
return {
message: () => `expected at least 2 elements, got ${received.length}`,
pass: false,
};
}

const pass = received[received.length - 1] > lastResult;
const r = lastResult;

const message = () =>
this.isNot
? `expected ${received[received.length - 1]} to be equal or lesser than ${r}`
: `expected ${received[received.length - 1]} to be greater than ${r}`;

lastResult = received[received.length - 1];

return {
message,
pass,
};
},
});
});

describe('useStructureRev', () => {
afterEach(() => resetCounters());

it('should increment only when relevant fields in frame change', () => {
let frames: DataFrame[] = [toDataFrame({ fields: [{ name: 'time', type: FieldType.time, values: [1, 2, 3] }] })];
const { result, rerender } = renderHook((frames) => useStructureRev(frames), { initialProps: frames });
startCounters(result.current);

// When changing number of frames, the structureRev should increment
frames = [...frames, toDataFrame({ fields: [{ name: 'time', type: FieldType.time, values: [1, 2, 3] }] })];
rerender(frames);
expect(result.all).toHaveIncremented();

// Changing RefId should not increment the structure revision
frames[0] = toDataFrame({
refId: 'A',
fields: [{ name: 'time', type: FieldType.time, values: [1, 2, 3] }],
});
rerender([...frames]);
expect(result.all).not.toHaveIncremented();

// Changing frame name should increment the structure revision
frames[0] = toDataFrame({
refId: 'A',
name: 'Some Name',
fields: [{ name: 'time', type: FieldType.time, values: [1, 2, 3] }],
});
rerender([...frames]);
expect(result.all).toHaveIncremented();

// Changing frame's fields number should increment the structure revision
frames[0] = toDataFrame({
refId: 'A',
name: 'Some Name',
fields: [
{ name: 'time', type: FieldType.time, values: [1, 2, 3] },
{ name: 'value', type: FieldType.number, values: [1, 2, 3] },
],
});
rerender([...frames]);
expect(result.all).toHaveIncremented();

// Changing a frame's field's config should increment the structure revision
frames[0] = toDataFrame({
refId: 'A',
name: 'Some Name',
fields: [
{ name: 'time', type: FieldType.time, values: [1, 2, 3] },
{ name: 'value', type: FieldType.number, values: [1, 2, 3], config: { unit: 'ms' } },
],
});
rerender([...frames]);
expect(result.all).toHaveIncremented();

// Changing a frame's field's name should increment the structure revision
frames[0] = toDataFrame({
refId: 'A',
name: 'Some Name',
fields: [
{ name: 'time', type: FieldType.time, values: [1, 2, 3] },
{ name: 'value, but with a different name', type: FieldType.number, values: [1, 2, 3], config: { unit: 'ms' } },
],
});
rerender([...frames]);
expect(result.all).toHaveIncremented();
});
});
20 changes: 20 additions & 0 deletions public/app/features/explore/Graph/useStructureRev.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { useMemo } from 'react';
import { useCounter, usePrevious } from 'react-use';

import { DataFrame, compareArrayValues, compareDataFrameStructures } from '@grafana/data';

export function useStructureRev(frames: DataFrame[]) {
const [structureRev, { inc }] = useCounter(0);
const previousFrames = usePrevious(frames);

// We need to increment structureRev when the number of series changes.
// the function passed to useMemo runs during rendering, so when we get a different
// amount of data, structureRev is incremented before we render it
useMemo(() => {
if (previousFrames && !compareArrayValues(frames, previousFrames, compareDataFrameStructures)) {
inc();
}
}, [frames, previousFrames, inc]);

return structureRev;
}

0 comments on commit 6b6b733

Please sign in to comment.