From 1146ac790c73ac45cf5c471ad5811b775c54c56c Mon Sep 17 00:00:00 2001 From: Paul Marbach Date: Wed, 10 Dec 2025 12:37:05 -0500 Subject: [PATCH] Sparkline: Prevent infinite loop when rendering a sparkline with a single value (#114203) * Sparkline: Prevent infinite loop when rendering a sparkline with a single value * some tests for this case * refactor out utils, experiment with getting highlightIndex working * add comments throughout for #112977 * remove unused import * Update Sparkline.test.tsx * fix points mode rendering --- eslint-suppressions.json | 5 - .../grafana-data/src/field/fieldDisplay.ts | 60 +++++- .../components/RadialGauge/RadialGauge.tsx | 9 +- .../components/Sparkline/Sparkline.test.tsx | 44 ++++ .../src/components/Sparkline/Sparkline.tsx | 198 ++---------------- .../src/components/Sparkline/utils.ts | 148 ++++++++++++- public/locales/en-US/grafana.json | 7 + 7 files changed, 274 insertions(+), 197 deletions(-) diff --git a/eslint-suppressions.json b/eslint-suppressions.json index c5169c289b0..d1c9af43a95 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -786,11 +786,6 @@ "count": 13 } }, - "packages/grafana-ui/src/components/Sparkline/Sparkline.tsx": { - "react-prefer-function-component/react-prefer-function-component": { - "count": 1 - } - }, "packages/grafana-ui/src/components/Table/Cells/TableCell.tsx": { "@typescript-eslint/consistent-type-assertions": { "count": 3 diff --git a/packages/grafana-data/src/field/fieldDisplay.ts b/packages/grafana-data/src/field/fieldDisplay.ts index c4423173da3..3d82f571926 100644 --- a/packages/grafana-data/src/field/fieldDisplay.ts +++ b/packages/grafana-data/src/field/fieldDisplay.ts @@ -190,10 +190,62 @@ export const getFieldDisplayValues = (options: GetFieldDisplayValuesOptions): Fi y: dataFrame.fields[i], x: timeField, }; - if (calc === ReducerID.last) { - sparkline.highlightIndex = sparkline.y.values.length - 1; - } else if (calc === ReducerID.first) { - sparkline.highlightIndex = 0; + let highlightIdx: number | undefined = (() => { + switch (calc) { + case ReducerID.last: + return sparkline.y.values.length - 1; + case ReducerID.first: + return 0; + // TODO: #112977 enable more reducers for highlight index + // case ReducerID.lastNotNull: { + // for (let k = sparkline.y.values.length - 1; k >= 0; k--) { + // const v = sparkline.y.values[k]; + // if (v !== null && v !== undefined && !Number.isNaN(v)) { + // return k; + // } + // } + // return; + // } + // case ReducerID.firstNotNull: { + // for (let k = 0; k < sparkline.y.values.length; k++) { + // const v = sparkline.y.values[k]; + // if (v !== null && v !== undefined && !Number.isNaN(v)) { + // return k; + // } + // } + // return; + // } + // case ReducerID.min: { + // let minIdx = -1; + // let prevMin = Infinity; + // for (let k = 0; k < sparkline.y.values.length; k++) { + // const v = sparkline.y.values[k]; + // if (v !== null && v !== undefined && !Number.isNaN(v) && v < prevMin) { + // prevMin = v; + // minIdx = k; + // } + // } + // return minIdx >= 0 ? minIdx : undefined; + // } + // case ReducerID.max: { + // let maxIdx = -1; + // let prevMax = -Infinity; + // for (let k = 0; k < sparkline.y.values.length; k++) { + // const v = sparkline.y.values[k]; + // if (v !== null && v !== undefined && !Number.isNaN(v) && v > prevMax) { + // prevMax = v; + // maxIdx = k; + // } + // } + // return maxIdx >= 0 ? maxIdx : undefined; + // } + default: + return; + } + })(); + + if (typeof highlightIdx === 'number') { + sparkline.highlightIndex = highlightIdx; } } diff --git a/packages/grafana-ui/src/components/RadialGauge/RadialGauge.tsx b/packages/grafana-ui/src/components/RadialGauge/RadialGauge.tsx index 96e8b0f50f3..18147e0cac5 100644 --- a/packages/grafana-ui/src/components/RadialGauge/RadialGauge.tsx +++ b/packages/grafana-ui/src/components/RadialGauge/RadialGauge.tsx @@ -2,7 +2,13 @@ import { css, cx } from '@emotion/css'; import { isNumber } from 'lodash'; import { useId } from 'react'; -import { DisplayValueAlignmentFactors, FieldDisplay, getDisplayProcessor, GrafanaTheme2 } from '@grafana/data'; +import { + DisplayValueAlignmentFactors, + FieldDisplay, + getDisplayProcessor, + GrafanaTheme2, + TimeRange, +} from '@grafana/data'; import { t } from '@grafana/i18n'; import { useStyles2, useTheme2 } from '../../themes/ThemeContext'; @@ -66,6 +72,7 @@ export interface RadialGaugeProps { showScaleLabels?: boolean; /** For data links */ onClick?: React.MouseEventHandler; + timeRange?: TimeRange; } export type RadialGradientMode = 'none' | 'auto'; diff --git a/packages/grafana-ui/src/components/Sparkline/Sparkline.test.tsx b/packages/grafana-ui/src/components/Sparkline/Sparkline.test.tsx index 9ad678132e9..91f3d9945f3 100644 --- a/packages/grafana-ui/src/components/Sparkline/Sparkline.test.tsx +++ b/packages/grafana-ui/src/components/Sparkline/Sparkline.test.tsx @@ -27,4 +27,48 @@ describe('Sparkline', () => { render() ).not.toThrow(); }); + + it('should not throw an error if there is a single value', () => { + const sparkline: FieldSparkline = { + x: { + name: 'x', + values: [1679839200000], + type: FieldType.time, + config: {}, + }, + y: { + name: 'y', + values: [1], + type: FieldType.number, + config: {}, + state: { + range: { min: 1, max: 1, delta: 0 }, + }, + }, + }; + expect(() => + render() + ).not.toThrow(); + }); + + it('should not throw an error if there are no values', () => { + const sparkline: FieldSparkline = { + x: { + name: 'x', + values: [], + type: FieldType.time, + config: {}, + }, + y: { + name: 'y', + values: [], + type: FieldType.number, + config: {}, + state: {}, + }, + }; + expect(() => + render() + ).not.toThrow(); + }); }); diff --git a/packages/grafana-ui/src/components/Sparkline/Sparkline.tsx b/packages/grafana-ui/src/components/Sparkline/Sparkline.tsx index 8de8ed1b893..68b1a5a832f 100644 --- a/packages/grafana-ui/src/components/Sparkline/Sparkline.tsx +++ b/packages/grafana-ui/src/components/Sparkline/Sparkline.tsx @@ -1,32 +1,13 @@ -import { isEqual } from 'lodash'; -import { PureComponent } from 'react'; -import { AlignedData, Range } from 'uplot'; +import React, { memo } from 'react'; -import { - compareDataFrameStructures, - DataFrame, - Field, - FieldConfig, - FieldSparkline, - FieldType, - getFieldColorModeForField, - nullToValue, -} from '@grafana/data'; -import { - AxisPlacement, - GraphDrawStyle, - GraphFieldConfig, - VisibilityMode, - ScaleDirection, - ScaleOrientation, -} from '@grafana/schema'; +import { FieldConfig, FieldSparkline } from '@grafana/data'; +import { GraphFieldConfig } from '@grafana/schema'; import { Themeable2 } from '../../types/theme'; import { UPlotChart } from '../uPlot/Plot'; -import { UPlotConfigBuilder } from '../uPlot/config/UPlotConfigBuilder'; import { preparePlotData2, getStackingGroups } from '../uPlot/utils'; -import { getYRange, preparePlotFrame } from './utils'; +import { prepareSeries, prepareConfig } from './utils'; export interface SparklineProps extends Themeable2 { width: number; @@ -35,169 +16,18 @@ export interface SparklineProps extends Themeable2 { sparkline: FieldSparkline; } -interface State { - data: AlignedData; - alignedDataFrame: DataFrame; - configBuilder: UPlotConfigBuilder; -} +export const Sparkline: React.FC = memo((props) => { + const { sparkline, config: fieldConfig, theme, width, height } = props; -const defaultConfig: GraphFieldConfig = { - drawStyle: GraphDrawStyle.Line, - showPoints: VisibilityMode.Auto, - axisPlacement: AxisPlacement.Hidden, - pointSize: 2, -}; - -/** @internal */ -export class Sparkline extends PureComponent { - constructor(props: SparklineProps) { - super(props); - - const alignedDataFrame = preparePlotFrame(props.sparkline, props.config); - - this.state = { - data: preparePlotData2(alignedDataFrame, getStackingGroups(alignedDataFrame)), - alignedDataFrame, - configBuilder: this.prepareConfig(alignedDataFrame), - }; + const { frame: alignedDataFrame, warning } = prepareSeries(sparkline, fieldConfig); + if (warning) { + return null; } - static getDerivedStateFromProps(props: SparklineProps, state: State) { - const _frame = preparePlotFrame(props.sparkline, props.config); - const frame = nullToValue(_frame); - if (!frame) { - return { ...state }; - } + const data = preparePlotData2(alignedDataFrame, getStackingGroups(alignedDataFrame)); + const configBuilder = prepareConfig(sparkline, alignedDataFrame, theme); - return { - ...state, - data: preparePlotData2(frame, getStackingGroups(frame)), - alignedDataFrame: frame, - }; - } + return ; +}); - componentDidUpdate(prevProps: SparklineProps, prevState: State) { - const { alignedDataFrame } = this.state; - - if (!alignedDataFrame) { - return; - } - - let rebuildConfig = false; - - if (prevProps.sparkline !== this.props.sparkline) { - const isStructureChanged = !compareDataFrameStructures(this.state.alignedDataFrame, prevState.alignedDataFrame); - const isRangeChanged = !isEqual( - alignedDataFrame.fields[1].state?.range, - prevState.alignedDataFrame.fields[1].state?.range - ); - rebuildConfig = isStructureChanged || isRangeChanged; - } else { - rebuildConfig = !isEqual(prevProps.config, this.props.config); - } - - if (rebuildConfig) { - this.setState({ configBuilder: this.prepareConfig(alignedDataFrame) }); - } - } - - getYRange(field: Field): Range.MinMax { - return getYRange(field, this.state.alignedDataFrame); - } - - prepareConfig(data: DataFrame) { - const { theme } = this.props; - const builder = new UPlotConfigBuilder(); - - builder.setCursor({ - show: false, - x: false, // no crosshairs - y: false, - }); - - // X is the first field in the alligned frame - const xField = data.fields[0]; - builder.addScale({ - scaleKey: 'x', - orientation: ScaleOrientation.Horizontal, - direction: ScaleDirection.Right, - isTime: false, //xField.type === FieldType.time, - range: () => { - const { sparkline } = this.props; - if (sparkline.x) { - if (sparkline.timeRange && sparkline.x.type === FieldType.time) { - return [sparkline.timeRange.from.valueOf(), sparkline.timeRange.to.valueOf()]; - } - const vals = sparkline.x.values; - return [vals[0], vals[vals.length - 1]]; - } - return [0, sparkline.y.values.length - 1]; - }, - }); - - builder.addAxis({ - scaleKey: 'x', - theme, - placement: AxisPlacement.Hidden, - }); - - for (let i = 0; i < data.fields.length; i++) { - const field = data.fields[i]; - const config: FieldConfig = field.config; - const customConfig: GraphFieldConfig = { - ...defaultConfig, - ...config.custom, - }; - - if (field === xField || field.type !== FieldType.number) { - continue; - } - - const scaleKey = config.unit || '__fixed'; - builder.addScale({ - scaleKey, - orientation: ScaleOrientation.Vertical, - direction: ScaleDirection.Up, - range: () => this.getYRange(field), - }); - - builder.addAxis({ - scaleKey, - theme, - placement: AxisPlacement.Hidden, - }); - - const colorMode = getFieldColorModeForField(field); - const seriesColor = colorMode.getCalculator(field, theme)(0, 0); - const pointsMode = - customConfig.drawStyle === GraphDrawStyle.Points ? VisibilityMode.Always : customConfig.showPoints; - - builder.addSeries({ - pxAlign: false, - scaleKey, - theme, - colorMode, - thresholds: config.thresholds, - drawStyle: customConfig.drawStyle!, - lineColor: customConfig.lineColor ?? seriesColor, - lineWidth: customConfig.lineWidth, - lineInterpolation: customConfig.lineInterpolation, - showPoints: pointsMode, - pointSize: customConfig.pointSize, - fillOpacity: customConfig.fillOpacity, - fillColor: customConfig.fillColor, - lineStyle: customConfig.lineStyle, - gradientMode: customConfig.gradientMode, - spanNulls: customConfig.spanNulls, - }); - } - - return builder; - } - - render() { - const { data, configBuilder } = this.state; - const { width, height } = this.props; - return ; - } -} +Sparkline.displayName = 'Sparkline'; diff --git a/packages/grafana-ui/src/components/Sparkline/utils.ts b/packages/grafana-ui/src/components/Sparkline/utils.ts index 5c2afd8a1cb..fa07cafb604 100644 --- a/packages/grafana-ui/src/components/Sparkline/utils.ts +++ b/packages/grafana-ui/src/components/Sparkline/utils.ts @@ -1,16 +1,29 @@ import { Range } from 'uplot'; import { + applyNullInsertThreshold, DataFrame, + Field, FieldConfig, FieldSparkline, FieldType, + getFieldColorModeForField, + GrafanaTheme2, isLikelyAscendingVector, + nullToValue, sortDataFrame, - applyNullInsertThreshold, - Field, } from '@grafana/data'; -import { GraphFieldConfig } from '@grafana/schema'; +import { t } from '@grafana/i18n'; +import { + AxisPlacement, + GraphDrawStyle, + GraphFieldConfig, + VisibilityMode, + ScaleDirection, + ScaleOrientation, +} from '@grafana/schema'; + +import { UPlotConfigBuilder } from '../uPlot/config/UPlotConfigBuilder'; /** @internal * Given a sparkline config returns a DataFrame ready to be turned into Plot data set @@ -85,3 +98,132 @@ export function getYRange(field: Field, alignedFrame: DataFrame): Range.MinMax { return [min, max]; } + +// TODO: #112977 enable highlight index +// const HIGHLIGHT_IDX_POINT_SIZE = 6; + +const defaultConfig: GraphFieldConfig = { + drawStyle: GraphDrawStyle.Line, + showPoints: VisibilityMode.Auto, + axisPlacement: AxisPlacement.Hidden, + pointSize: 2, +}; + +export const prepareSeries = ( + sparkline: FieldSparkline, + fieldConfig?: FieldConfig +): { frame: DataFrame; warning?: string } => { + const frame = nullToValue(preparePlotFrame(sparkline, fieldConfig)); + if (frame.fields.some((f) => f.values.length <= 1)) { + return { + warning: t( + 'grafana-ui.components.sparkline.warning.too-few-values', + 'Sparkline requires at least two values to render.' + ), + frame, + }; + } + return { frame }; +}; + +export const prepareConfig = ( + sparkline: FieldSparkline, + dataFrame: DataFrame, + theme: GrafanaTheme2 +): UPlotConfigBuilder => { + const builder = new UPlotConfigBuilder(); + // const rangePad = HIGHLIGHT_IDX_POINT_SIZE / 2; + + builder.setCursor({ + show: false, + x: false, // no crosshairs + y: false, + }); + + // X is the first field in the aligned frame + const xField = dataFrame.fields[0]; + builder.addScale({ + scaleKey: 'x', + orientation: ScaleOrientation.Horizontal, + direction: ScaleDirection.Right, + isTime: false, // xField.type === FieldType.time, + range: () => { + if (sparkline.x) { + if (sparkline.timeRange && sparkline.x.type === FieldType.time) { + return [sparkline.timeRange.from.valueOf(), sparkline.timeRange.to.valueOf()]; + } + const vals = sparkline.x.values; + return [vals[0], vals[vals.length - 1]]; + } + return [0, sparkline.y.values.length - 1]; + }, + }); + + builder.addAxis({ + scaleKey: 'x', + theme, + placement: AxisPlacement.Hidden, + }); + + for (let i = 0; i < dataFrame.fields.length; i++) { + const field = dataFrame.fields[i]; + const config: FieldConfig = field.config; + const customConfig: GraphFieldConfig = { + ...defaultConfig, + ...config.custom, + }; + + if (field === xField || field.type !== FieldType.number) { + continue; + } + + const scaleKey = config.unit || '__fixed'; + builder.addScale({ + scaleKey, + orientation: ScaleOrientation.Vertical, + direction: ScaleDirection.Up, + range: () => getYRange(field, dataFrame), + }); + + builder.addAxis({ + scaleKey, + theme, + placement: AxisPlacement.Hidden, + }); + + const colorMode = getFieldColorModeForField(field); + const seriesColor = colorMode.getCalculator(field, theme)(0, 0); + // TODO: #112977 enable highlight index and adjust padding accordingly + // const hasHighlightIndex = typeof sparkline.highlightIndex === 'number'; + // if (hasHighlightIndex) { + // builder.setPadding([rangePad, rangePad, rangePad, rangePad]); + // } + const pointsMode = + customConfig.drawStyle === GraphDrawStyle.Points // || hasHighlightIndex + ? VisibilityMode.Always + : customConfig.showPoints; + + builder.addSeries({ + pxAlign: false, + scaleKey, + theme, + colorMode, + thresholds: config.thresholds, + drawStyle: customConfig.drawStyle!, + lineColor: customConfig.lineColor ?? seriesColor, + lineWidth: customConfig.lineWidth, + lineInterpolation: customConfig.lineInterpolation, + showPoints: pointsMode, + // TODO: #112977 enable highlight index + pointSize: /* hasHighlightIndex ? HIGHLIGHT_IDX_POINT_SIZE : */ customConfig.pointSize, + // pointsFilter: hasHighlightIndex ? [sparkline.highlightIndex!] : undefined, + fillOpacity: customConfig.fillOpacity, + fillColor: customConfig.fillColor, + lineStyle: customConfig.lineStyle, + gradientMode: customConfig.gradientMode, + spanNulls: customConfig.spanNulls, + }); + } + + return builder; +}; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index d6ecdd24882..919b5f1e1ce 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -8827,6 +8827,13 @@ "aria-label-default": "Pick a color", "aria-label-selected-color": "{{colorLabel}} color" }, + "components": { + "sparkline": { + "warning": { + "too-few-values": "Sparkline requires at least two values to render." + } + } + }, "confirm-button": { "aria-label-delete": "Delete", "cancel": "Cancel",