From da8d4ecb19096ce6f125c693217bc89ef8602245 Mon Sep 17 00:00:00 2001 From: saravanakumardb1 Date: Wed, 27 May 2026 18:43:18 -0700 Subject: [PATCH] fix(charts): drop NaN/Infinity from series; DRY-up compactNumber MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit pass found two latent issues: 1. **NaN/Infinity broke the SVG path.** `LineChart` mapped raw values through `yScale()` without sanitising, so any non-finite input emitted a literal 'NaN' in the path `d` attribute and silently broke the visible stroke for the whole series. Same shape in `AreaChart`. 2. **`compactNumber()` was duplicated.** Three identical copies lived in LineChart / BarChart / AreaChart. Fixes (all in `utils.ts`): + `compactNumber(v)` now exported (returns '' for non-finite) + `filterFinite(values)` returns `[index, value]` pairs, keeping the original X-axis spacing for the surviving points Behavioural changes: - LineChart `series` containing NaN/Infinity → path skips those points cleanly. Series of *entirely* non-finite values render nothing (was: a fully NaN-poisoned path). - AreaChart `values` containing NaN/Infinity → same. - BarChart unchanged (was already safe via `extent`). Tests: 19 \u2192 23 (4 new regression cases) utils \u00b7 compactNumber k-suffix + non-finite handling utils \u00b7 filterFinite preserves original indices LineChart \u00b7 NaN/Infinity never appear in path `d` LineChart \u00b7 all-non-finite series renders zero --- packages/charts/src/AreaChart.tsx | 12 +++---- packages/charts/src/BarChart.tsx | 6 +--- packages/charts/src/LineChart.tsx | 15 ++++---- packages/charts/src/__tests__/charts.test.tsx | 36 ++++++++++++++++++- packages/charts/src/utils.ts | 21 +++++++++++ 5 files changed, 68 insertions(+), 22 deletions(-) diff --git a/packages/charts/src/AreaChart.tsx b/packages/charts/src/AreaChart.tsx index a18818e7..f99a7d14 100644 --- a/packages/charts/src/AreaChart.tsx +++ b/packages/charts/src/AreaChart.tsx @@ -1,5 +1,5 @@ import { useId, type CSSProperties } from 'react'; -import { extent, linearScale, smoothPath } from './utils.js'; +import { compactNumber, extent, filterFinite, linearScale, smoothPath } from './utils.js'; export interface AreaChartProps { /** Series Y-values. */ @@ -42,13 +42,15 @@ export function AreaChart({ const innerW = Math.max(0, width - padL - padR); const innerH = Math.max(0, height - padT - padB); - const [yMin, yMax] = extent(values); + const finitePairs = filterFinite(values); + const finiteValues = finitePairs.map(([, v]) => v); + const [yMin, yMax] = extent(finiteValues); const yMinDisplay = Math.min(0, yMin); const yScale = linearScale(yMinDisplay, yMax, innerH, 0); const xScale = (i: number) => values.length > 1 ? (i / (values.length - 1)) * innerW : innerW / 2; - const points: Array<[number, number]> = values.map((v, i) => [ + const points: Array<[number, number]> = finitePairs.map(([i, v]) => [ xScale(i), yScale(v), ]); @@ -115,7 +117,3 @@ export function AreaChart({ ); } -function compactNumber(v: number): string { - if (Math.abs(v) >= 1000) return `${(v / 1000).toFixed(1)}k`; - return v.toFixed(Math.abs(v) < 1 ? 2 : 0); -} diff --git a/packages/charts/src/BarChart.tsx b/packages/charts/src/BarChart.tsx index 586e28e5..6deaef12 100644 --- a/packages/charts/src/BarChart.tsx +++ b/packages/charts/src/BarChart.tsx @@ -1,5 +1,5 @@ import { useId, type CSSProperties } from 'react'; -import { extent, linearScale } from './utils.js'; +import { compactNumber, extent, linearScale } from './utils.js'; export interface BarDatum { /** Stable id — React key + accessible label. */ @@ -139,7 +139,3 @@ export function BarChart({ ); } -function compactNumber(v: number): string { - if (Math.abs(v) >= 1000) return `${(v / 1000).toFixed(1)}k`; - return v.toFixed(Math.abs(v) < 1 ? 2 : 0); -} diff --git a/packages/charts/src/LineChart.tsx b/packages/charts/src/LineChart.tsx index 70920dcc..c682edbf 100644 --- a/packages/charts/src/LineChart.tsx +++ b/packages/charts/src/LineChart.tsx @@ -1,5 +1,5 @@ import { useId, type CSSProperties } from 'react'; -import { extent, linearScale, smoothPath } from './utils.js'; +import { compactNumber, extent, filterFinite, linearScale, smoothPath } from './utils.js'; export interface LineSeries { /** Stable id — React key + accessible series label. */ @@ -112,10 +112,11 @@ export function LineChart({ ))} {series.map((s, idx) => { const colour = s.color ?? TOKEN_PALETTE[idx % TOKEN_PALETTE.length]!; - const points: Array<[number, number]> = s.values.map((v, i) => [ - xScale(i), - yScale(v), - ]); + // Drop NaN / Infinity so the SVG path stays well-formed. + const points: Array<[number, number]> = filterFinite(s.values).map( + ([i, v]) => [xScale(i), yScale(v)], + ); + if (points.length === 0) return null; const d = smooth ? smoothPath(points) : `M ${points.map(([x, y]) => `${x} ${y}`).join(' L ')}`; @@ -154,7 +155,3 @@ function niceTicks(lo: number, hi: number, n: number): number[] { return Array.from({ length: n }, (_, i) => lo + step * i); } -function compactNumber(v: number): string { - if (Math.abs(v) >= 1000) return `${(v / 1000).toFixed(1)}k`; - return v.toFixed(Math.abs(v) < 1 ? 2 : 0); -} diff --git a/packages/charts/src/__tests__/charts.test.tsx b/packages/charts/src/__tests__/charts.test.tsx index 3760f497..1dc0634a 100644 --- a/packages/charts/src/__tests__/charts.test.tsx +++ b/packages/charts/src/__tests__/charts.test.tsx @@ -6,7 +6,7 @@ import { BarChart } from '../BarChart.js'; import { AreaChart } from '../AreaChart.js'; import { Donut } from '../Donut.js'; import { Gauge } from '../Gauge.js'; -import { extent, linearScale, smoothPath } from '../utils.js'; +import { compactNumber, extent, filterFinite, linearScale, smoothPath } from '../utils.js'; beforeEach(() => cleanup()); @@ -31,6 +31,22 @@ describe('utils', () => { expect(smoothPath([[1, 2]])).toBe('M 1 2'); expect(smoothPath([[0, 0], [10, 10]])).toMatch(/^M 0 0 C/); }); + + it('compactNumber renders k-suffix, fractions, and empty for non-finite', () => { + expect(compactNumber(2400)).toBe('2.4k'); + expect(compactNumber(0.5)).toBe('0.50'); + expect(compactNumber(7)).toBe('7'); + expect(compactNumber(NaN)).toBe(''); + expect(compactNumber(Infinity)).toBe(''); + }); + + it('filterFinite drops NaN/Infinity but preserves original indices', () => { + expect(filterFinite([1, NaN, 3, Infinity, 5])).toEqual([ + [0, 1], + [2, 3], + [4, 5], + ]); + }); }); describe('LineChart', () => { @@ -57,6 +73,24 @@ describe('LineChart', () => { render(); expect(screen.getByTestId('bl-line-chart').querySelector('title')?.textContent).toBe('My label'); }); + + it('drops NaN / Infinity from a series rather than emitting NaN in the SVG path', () => { + render( + , + ); + const path = screen.getByTestId('bl-line-chart-series').querySelector('path'); + const d = path?.getAttribute('d') ?? ''; + expect(d).not.toMatch(/NaN/); + expect(d).not.toMatch(/Infinity/); + expect(d.length).toBeGreaterThan(0); + }); + + it('renders nothing for a series of entirely non-finite values', () => { + render(); + expect(screen.queryByTestId('bl-line-chart-series')).toBeNull(); + }); }); describe('BarChart', () => { diff --git a/packages/charts/src/utils.ts b/packages/charts/src/utils.ts index ef38cece..2052d8f7 100644 --- a/packages/charts/src/utils.ts +++ b/packages/charts/src/utils.ts @@ -41,6 +41,27 @@ export function formatNumber( } } +/** Compact numeric formatter — `1234` → `1.2k`, `0.5` → `0.50`. */ +export function compactNumber(v: number): string { + if (!Number.isFinite(v)) return ''; + if (Math.abs(v) >= 1000) return `${(v / 1000).toFixed(1)}k`; + return v.toFixed(Math.abs(v) < 1 ? 2 : 0); +} + +/** + * Drop non-finite (NaN / Infinity) values from a numeric series, + * preserving the original indices. Returns the `[index, value]` pairs + * so callers can keep their X-axis spacing intact. + */ +export function filterFinite(values: readonly number[]): Array<[number, number]> { + const out: Array<[number, number]> = []; + for (let i = 0; i < values.length; i++) { + const v = values[i]!; + if (Number.isFinite(v)) out.push([i, v]); + } + return out; +} + /** Build a smooth catmull-rom path through `pts`. */ export function smoothPath(pts: ReadonlyArray<[number, number]>): string { if (pts.length === 0) return '';