fix(charts): drop NaN/Infinity from series; DRY-up compactNumber
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 <g>
This commit is contained in:
parent
d57ed9b878
commit
da8d4ecb19
@ -1,5 +1,5 @@
|
|||||||
import { useId, type CSSProperties } from 'react';
|
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 {
|
export interface AreaChartProps {
|
||||||
/** Series Y-values. */
|
/** Series Y-values. */
|
||||||
@ -42,13 +42,15 @@ export function AreaChart({
|
|||||||
const innerW = Math.max(0, width - padL - padR);
|
const innerW = Math.max(0, width - padL - padR);
|
||||||
const innerH = Math.max(0, height - padT - padB);
|
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 yMinDisplay = Math.min(0, yMin);
|
||||||
const yScale = linearScale(yMinDisplay, yMax, innerH, 0);
|
const yScale = linearScale(yMinDisplay, yMax, innerH, 0);
|
||||||
const xScale = (i: number) =>
|
const xScale = (i: number) =>
|
||||||
values.length > 1 ? (i / (values.length - 1)) * innerW : innerW / 2;
|
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),
|
xScale(i),
|
||||||
yScale(v),
|
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);
|
|
||||||
}
|
|
||||||
|
|||||||
@ -1,5 +1,5 @@
|
|||||||
import { useId, type CSSProperties } from 'react';
|
import { useId, type CSSProperties } from 'react';
|
||||||
import { extent, linearScale } from './utils.js';
|
import { compactNumber, extent, linearScale } from './utils.js';
|
||||||
|
|
||||||
export interface BarDatum {
|
export interface BarDatum {
|
||||||
/** Stable id — React key + accessible label. */
|
/** 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);
|
|
||||||
}
|
|
||||||
|
|||||||
@ -1,5 +1,5 @@
|
|||||||
import { useId, type CSSProperties } from 'react';
|
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 {
|
export interface LineSeries {
|
||||||
/** Stable id — React key + accessible series label. */
|
/** Stable id — React key + accessible series label. */
|
||||||
@ -112,10 +112,11 @@ export function LineChart({
|
|||||||
))}
|
))}
|
||||||
{series.map((s, idx) => {
|
{series.map((s, idx) => {
|
||||||
const colour = s.color ?? TOKEN_PALETTE[idx % TOKEN_PALETTE.length]!;
|
const colour = s.color ?? TOKEN_PALETTE[idx % TOKEN_PALETTE.length]!;
|
||||||
const points: Array<[number, number]> = s.values.map((v, i) => [
|
// Drop NaN / Infinity so the SVG path stays well-formed.
|
||||||
xScale(i),
|
const points: Array<[number, number]> = filterFinite(s.values).map(
|
||||||
yScale(v),
|
([i, v]) => [xScale(i), yScale(v)],
|
||||||
]);
|
);
|
||||||
|
if (points.length === 0) return null;
|
||||||
const d = smooth
|
const d = smooth
|
||||||
? smoothPath(points)
|
? smoothPath(points)
|
||||||
: `M ${points.map(([x, y]) => `${x} ${y}`).join(' L ')}`;
|
: `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);
|
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);
|
|
||||||
}
|
|
||||||
|
|||||||
@ -6,7 +6,7 @@ import { BarChart } from '../BarChart.js';
|
|||||||
import { AreaChart } from '../AreaChart.js';
|
import { AreaChart } from '../AreaChart.js';
|
||||||
import { Donut } from '../Donut.js';
|
import { Donut } from '../Donut.js';
|
||||||
import { Gauge } from '../Gauge.js';
|
import { Gauge } from '../Gauge.js';
|
||||||
import { extent, linearScale, smoothPath } from '../utils.js';
|
import { compactNumber, extent, filterFinite, linearScale, smoothPath } from '../utils.js';
|
||||||
|
|
||||||
beforeEach(() => cleanup());
|
beforeEach(() => cleanup());
|
||||||
|
|
||||||
@ -31,6 +31,22 @@ describe('utils', () => {
|
|||||||
expect(smoothPath([[1, 2]])).toBe('M 1 2');
|
expect(smoothPath([[1, 2]])).toBe('M 1 2');
|
||||||
expect(smoothPath([[0, 0], [10, 10]])).toMatch(/^M 0 0 C/);
|
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', () => {
|
describe('LineChart', () => {
|
||||||
@ -57,6 +73,24 @@ describe('LineChart', () => {
|
|||||||
render(<LineChart series={[{ id: 'a', values: [1] }]} ariaLabel="My label" />);
|
render(<LineChart series={[{ id: 'a', values: [1] }]} ariaLabel="My label" />);
|
||||||
expect(screen.getByTestId('bl-line-chart').querySelector('title')?.textContent).toBe('My label');
|
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(
|
||||||
|
<LineChart
|
||||||
|
series={[{ id: 'gappy', values: [1, NaN, 3, Infinity, 5] }]}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
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(<LineChart series={[{ id: 'broken', values: [NaN, NaN] }]} />);
|
||||||
|
expect(screen.queryByTestId('bl-line-chart-series')).toBeNull();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('BarChart', () => {
|
describe('BarChart', () => {
|
||||||
|
|||||||
@ -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`. */
|
/** Build a smooth catmull-rom path through `pts`. */
|
||||||
export function smoothPath(pts: ReadonlyArray<[number, number]>): string {
|
export function smoothPath(pts: ReadonlyArray<[number, number]>): string {
|
||||||
if (pts.length === 0) return '';
|
if (pts.length === 0) return '';
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user