diff --git a/.changeset/recharts-3-upgrade.md b/.changeset/recharts-3-upgrade.md new file mode 100644 index 0000000000..1bc825a12c --- /dev/null +++ b/.changeset/recharts-3-upgrade.md @@ -0,0 +1,10 @@ +--- +"@hyperdx/app": minor +--- + +chore(charts): upgrade Recharts from 2.13 to 3.x. Reworks chart event handlers +to the Recharts 3 event API (zoom-brush selection, click drill-down), replaces +the histogram's imperative `chart.setState` tooltip-pin hack with the controlled +`active`/`defaultIndex` Tooltip props, updates custom tooltip/shape typings +(`TooltipContentProps`, `BarProps`), and suppresses the browser focus ring that +Recharts 3's default `accessibilityLayer` shows when a chart is clicked. diff --git a/packages/app/package.json b/packages/app/package.json index a7fd52056c..e5b30daa15 100644 --- a/packages/app/package.json +++ b/packages/app/package.json @@ -84,10 +84,11 @@ "react-grid-layout": "^1.3.4", "react-hook-form": "^7.43.8", "react-hotkeys-hook": "^4.3.7", + "react-is": "^19.0.0", "react-json-tree": "^0.20.0", "react-markdown": "^10.1.0", "react-resizable": "^3.0.4", - "recharts": "^2.12.7", + "recharts": "^3.2.1", "rrweb": "2.0.0-alpha.8", "sass": "^1.54.8", "sql-formatter": "^15.7.2", diff --git a/packages/app/src/HDXMultiSeriesTimeChart.tsx b/packages/app/src/HDXMultiSeriesTimeChart.tsx index 5353efef94..e9d3715f0d 100644 --- a/packages/app/src/HDXMultiSeriesTimeChart.tsx +++ b/packages/app/src/HDXMultiSeriesTimeChart.tsx @@ -174,7 +174,10 @@ const HDXLineChartTooltip = withErrorBoundary( return ( - {payload + {/* Copy before sorting: Recharts 3 keeps the payload in its + Immer-backed store and freezes it, so an in-place sort throws + "this object has been frozen". */} + {[...payload] .sort((a: TooltipPayload, b: TooltipPayload) => b.value - a.value) .map((p: TooltipPayload) => { const previousKey = lineDataMap[p.dataKey]?.previousPeriodKey; @@ -316,7 +319,9 @@ const LegendRenderer = memo<{ } }); - return allSeriesPayload.sort((a, b) => { + // Copy before sorting: when this comes from Recharts' legend payload it is + // kept in the Immer-backed store and frozen, so an in-place sort throws. + return [...allSeriesPayload].sort((a, b) => { const keyA = lineDataMap[a.dataKey]?.currentPeriodKey ?? ''; const keyB = lineDataMap[b.dataKey]?.currentPeriodKey ?? ''; @@ -379,23 +384,115 @@ const LegendRenderer = memo<{ export const HARD_LINES_LIMIT = MAX_TIME_CHART_SERIES; +// Debounce (ms) for the chart's ResponsiveContainer resize observer. Without +// it the observer fires on every frame, and a resize → re-render → resize +// cycle can keep the chart (and the form controls around it in the tile +// editor) from ever settling. +const RESPONSIVE_CONTAINER_DEBOUNCE_MS = 50; + +/** One series entry in a click-frozen tooltip's payload. */ +type ActiveClickSeries = { + value?: number; + dataKey?: string; + name?: string; + /** + * Series color (matches the legend swatch). Part of the drill-down payload + * contract so a popover can show a per-series swatch. + */ + color?: string; +}; + +/** + * State for the click-frozen ("pinned") tooltip. Produced by MemoChart's + * onClick and consumed by DBTimeChart to render the drill-down popover. + */ +export type ActiveClickPayload = { + x: number; + y: number; + activeLabel: string; + xPerc: number; + yPerc: number; + activePayload?: ActiveClickSeries[]; +}; + +/** Series label shown in the legend, tooltip, and line `name`. */ +const getSeriesDisplayName = (ld: LineData) => ld.displayName || ld.dataKey; + +/** Normalize a chart event's active label (number | string) to a string. */ +const getActiveLabel = (state?: { + activeLabel?: string | number; +}): string | undefined => + state?.activeLabel != null ? String(state.activeLabel) : undefined; + +/** + * Build the per-series payload for a click-frozen tooltip from the data row at + * the clicked bucket. Only the visible series (legend selection + + * HARD_LINES_LIMIT) with a numeric value at that bucket are included, so the + * drill-down popover mirrors exactly what is drawn. Exported for unit testing. + */ +export function buildActiveClickSeries( + visibleLineData: LineData[], + activeRow: Record | undefined, +): ActiveClickSeries[] { + if (activeRow == null) return []; + return visibleLineData.flatMap(ld => { + const value = activeRow[ld.dataKey]; + if (typeof value !== 'number') return []; + return [ + { + dataKey: ld.dataKey, + name: getSeriesDisplayName(ld), + value, + color: ld.color, + }, + ]; + }); +} + +/** + * The series actually drawn on the chart: the first HARD_LINES_LIMIT of + * lineData, narrowed to the legend selection when one is active. The rendered + * lines and the drill-down click payload both derive from this same set so + * they never diverge. Exported for unit testing. + */ +export function getVisibleLineData( + lineData: LineData[], + selectedSeriesNames: Set | undefined, +): LineData[] { + const hasSelection = !!selectedSeriesNames && selectedSeriesNames.size > 0; + return lineData + .slice(0, HARD_LINES_LIMIT) + .filter( + ld => !hasSelection || selectedSeriesNames.has(getSeriesDisplayName(ld)), + ); +} + const StackedBarWithOverlap = (props: BarProps) => { - const { x, y, width, height, fill } = props; + const { x, y, width, fill } = props; + // `height` may arrive as a string, so coerce it to a number before the + // arithmetic below. + const height = + typeof props.height === 'number' ? props.height : Number(props.height ?? 0); // Add a tiny bit to the height to create overlap. Otherwise there's a gap return ( 0 ? height + 0.5 : 0} + height={height > 0 ? height + 0.5 : 0} fill={fill} /> ); }; type CaptureActiveDotProps = { - /** Shared ref the tooltip reads to find the series nearest the cursor. */ - captureRef: React.MutableRefObject>; + /** + * Called with each series' active-point pixel Y. This is a stable callback + * (not the ref itself) so Recharts, which stores this element's props in its + * Immer-backed store and freezes them, never freezes the underlying Map — + * the write happens on the ref captured in the callback's closure instead. + */ + onCapture: (dataKey: string, cy: number) => void; cx?: number; cy?: number; dataKey?: string | number; @@ -407,14 +504,14 @@ type CaptureActiveDotProps = { /** * Active dot for an Area series. Records the active point's pixel Y (`cy`) - * into `captureRef`, keyed by dataKey, then draws the same dot Recharts + * via `onCapture`, keyed by dataKey, then draws the same dot Recharts * renders by default. Recharts clones this element with the active-point * props (cx, cy, dataKey, r, fill, stroke, strokeWidth) during the render - * that precedes the tooltip, so the ref is current when the tooltip reads + * that precedes the tooltip, so the capture is current when the tooltip reads * it to find the series nearest the cursor. */ function CaptureActiveDot({ - captureRef, + onCapture, cx, cy, dataKey, @@ -427,8 +524,7 @@ function CaptureActiveDot({ // Written synchronously during render so the tooltip, which Recharts // renders after the graphical items in the same commit, reads the // current frame's positions rather than the previous frame's. - // eslint-disable-next-line react-hooks/refs - captureRef.current.set(String(dataKey), cy); + onCapture(String(dataKey), cy); } if (typeof cx !== 'number' || typeof cy !== 'number') { return null; @@ -496,8 +592,8 @@ export const MemoChart = memo(function MemoChart({ fitYAxisToData = false, }: { graphResults: any[]; - setIsClickActive: (v: any) => void; - isClickActive: any; + setIsClickActive: (v: ActiveClickPayload | undefined) => void; + isClickActive: ActiveClickPayload | undefined; dateRange: [Date, Date] | Readonly<[Date, Date]>; lineData: LineData[]; referenceLines?: React.ReactNode; @@ -538,6 +634,14 @@ export const MemoChart = memo(function MemoChart({ // Read during the same render that draws the active dots. const activePointYByKeyRef = useRef>(new Map()); + // Stable writer passed to the active-dot element instead of the ref itself. + // Recharts freezes the props of graphical-item elements in its Immer store; + // passing a callback (rather than the Map) keeps the mutation on the + // closed-over ref, which is never frozen. + const captureActivePointY = useCallback((dataKey: string, cy: number) => { + activePointYByKeyRef.current.set(dataKey, cy); + }, []); + // Key of the series whose line is nearest the cursor, lifted into state so // the chart can emphasize that line (thicker stroke) and fade the rest. // Set from the chart's mouse-move using the pixel Y the active dots captured @@ -553,33 +657,26 @@ export const MemoChart = memo(function MemoChart({ [displayType], ); - const lines = useMemo(() => { - const hasSelection = selectedSeriesNames && selectedSeriesNames.size > 0; - - const limitedGroupKeys = lineData - .map(ld => ld.dataKey) - .slice(0, HARD_LINES_LIMIT) - .filter((key, i) => { - const seriesName = lineData[i]?.displayName ?? key; - // If there's a selection, only show selected series - // If no selection, show all series - return !hasSelection || selectedSeriesNames.has(seriesName); - }); + const visibleLineData = useMemo( + () => getVisibleLineData(lineData, selectedSeriesNames), + [lineData, selectedSeriesNames], + ); + const lines = useMemo(() => { // When a series is nearest the cursor (only meaningful with more than one // line shown), thicken its line and fade the others so the eye lands on // the same series the tooltip bolds. Mirrors the legend's selected style // (thicker stroke) with a gentle fade that keeps the rest readable. const hasNearest = - limitedGroupKeys.length > 1 && + visibleLineData.length > 1 && nearestSeriesKey != null && - limitedGroupKeys.includes(nearestSeriesKey); + visibleLineData.some(ld => ld.dataKey === nearestSeriesKey); - return limitedGroupKeys.map(key => { - const lineDataIndex = lineData.findIndex(ld => ld.dataKey === key); - const color = lineData[lineDataIndex]?.color; - const strokeDasharray = lineData[lineDataIndex]?.isDashed ? '4 3' : '0'; - const seriesName = lineData[lineDataIndex]?.displayName ?? key; + return visibleLineData.map(ld => { + const key = ld.dataKey; + const color = ld.color; + const strokeDasharray = ld.isDashed ? '4 3' : '0'; + const seriesName = getSeriesDisplayName(ld); return displayType === 'stacked_bar' ? ( } + activeDot={} {...(isHovered ? { fill: 'none', strokeDasharray } : { @@ -618,12 +715,12 @@ export const MemoChart = memo(function MemoChart({ ); }); }, [ - lineData, + visibleLineData, displayType, id, isHovered, - selectedSeriesNames, nearestSeriesKey, + captureActivePointY, ]); const yAxisDomain: AxisDomain = useMemo(() => { @@ -688,6 +785,17 @@ export const MemoChart = memo(function MemoChart({ const sizeRef = useRef<[number, number]>([0, 0]); const [containerWidth, setContainerWidth] = useState(0); + // The chart's outer positioned container. Used to convert a pointer's + // viewport clientX into a stable container-relative X for measuring + // drag-to-zoom distance — a single origin that is always defined, unlike the + // chart's activeCoordinate (null off a data point) or a child SVG element's + // offsetX (relative to whichever bar/path is under the pointer). + const containerRef = useRef(null); + const getContainerX = useCallback((e?: { clientX?: number } | null) => { + if (e?.clientX == null || containerRef.current == null) return undefined; + return e.clientX - containerRef.current.getBoundingClientRect().left; + }, []); + // Recharts computes bar width from the smallest gap between ticks on a // numerical XAxis. With a single data point there are no gaps, so the // computed width is 0 and bars become invisible. Provide an explicit @@ -746,6 +854,11 @@ export const MemoChart = memo(function MemoChart({ // Set right before we trigger our own brush-zoom so the dateRange effect can // tell an internal zoom apart from an external time-range change. const justZoomedRef = useRef(false); + // Set on a completed brush-zoom so the synthetic click that follows mouseup + // is swallowed (instead of freezing a stale drill-down tooltip). Kept + // separate from justZoomedRef and consumed/cleared by onClick, because the + // dateRange effect may never run when the post-zoom range is value-equal. + const suppressNextClickRef = useRef(false); const prevDateRangeRef = useRef<[number, number] | null>(null); // Clear the reset-zoom affordance whenever the time range changes for a @@ -759,11 +872,17 @@ export const MemoChart = memo(function MemoChart({ const changed = prev == null || prev[0] !== from || prev[1] !== to; prevDateRangeRef.current = [from, to]; + // A brush-zoom sets justZoomedRef; consume it here so the range change it + // caused doesn't clear zoomOrigin. Clear it even when the range didn't + // actually change (a value-equal zoom), so it can't leak into a later + // unrelated range change and wrongly preserve a stale zoomOrigin. + const wasInternalZoom = justZoomedRef.current; + justZoomedRef.current = false; + if (!changed) { return; } - if (justZoomedRef.current) { - justZoomedRef.current = false; + if (wasInternalZoom) { return; } setZoomOrigin(null); @@ -822,7 +941,10 @@ export const MemoChart = memo(function MemoChart({ }, [annotations, xAxisDomain]); return ( -
+
{onTimeRangeSelect != null && zoomOrigin != null ? (