fix(data-viz): SSR-safe gradient id + NaN-safe ProgressRing
Two bugs in @bytelyst/data-viz@0.1.0 surfaced during a cross-repo audit:
1. Sparkline used `useMemo(() => \`bl-spark-${Math.random()...}\`)`
for its <linearGradient> id. Math.random() produces different values
during Next.js SSR vs client hydration, triggering React hydration
mismatches (and broken gradient refs on first paint). Swap for
React's `useId()` — deterministic across server + client.
Also: with a single-element series, `data.length > 0` was true but
the early-return branch left `lastX=lastY=0`, painting a stale dot
at the SVG origin. Tighten the guard to `data.length >= 2`.
2. ProgressRing's `Math.max(0, Math.min(1, value))` propagated NaN
when callers passed `NaN` or `Infinity` (e.g. division-by-zero
metrics) — producing "NaN percent" in the aria-label. Guard with
`Number.isFinite` first.
Regression tests cover all three cases — 17/17 passing.
Tests: pnpm -F @bytelyst/data-viz test → 17 passed
This commit is contained in:
parent
e29cc58ae7
commit
acb8f02bca
@ -37,7 +37,8 @@ export function ProgressRing({
|
|||||||
className,
|
className,
|
||||||
style,
|
style,
|
||||||
}: ProgressRingProps) {
|
}: ProgressRingProps) {
|
||||||
const clamped = Math.max(0, Math.min(1, value));
|
const safeValue = Number.isFinite(value) ? value : 0;
|
||||||
|
const clamped = Math.max(0, Math.min(1, safeValue));
|
||||||
const { radius, circumference, dashOffset } = useMemo(() => {
|
const { radius, circumference, dashOffset } = useMemo(() => {
|
||||||
const r = (size - thickness) / 2;
|
const r = (size - thickness) / 2;
|
||||||
const c = 2 * Math.PI * r;
|
const c = 2 * Math.PI * r;
|
||||||
|
|||||||
@ -1,4 +1,4 @@
|
|||||||
import { useMemo, type CSSProperties } from 'react';
|
import { useId, useMemo, type CSSProperties } from 'react';
|
||||||
|
|
||||||
export interface SparklineProps {
|
export interface SparklineProps {
|
||||||
/** Series values. Length 2+. */
|
/** Series values. Length 2+. */
|
||||||
@ -54,9 +54,7 @@ export function Sparkline({
|
|||||||
const y = pad + innerH - ((v - min) / range) * innerH;
|
const y = pad + innerH - ((v - min) / range) * innerH;
|
||||||
return [x, y] as const;
|
return [x, y] as const;
|
||||||
});
|
});
|
||||||
const path = pts
|
const path = pts.map(([x, y], i) => (i === 0 ? `M${x},${y}` : `L${x},${y}`)).join(' ');
|
||||||
.map(([x, y], i) => (i === 0 ? `M${x},${y}` : `L${x},${y}`))
|
|
||||||
.join(' ');
|
|
||||||
const areaPath =
|
const areaPath =
|
||||||
pts.length > 1
|
pts.length > 1
|
||||||
? `${path} L${pts[pts.length - 1]![0]},${height} L${pts[0]![0]},${height} Z`
|
? `${path} L${pts[pts.length - 1]![0]},${height} L${pts[0]![0]},${height} Z`
|
||||||
@ -65,10 +63,9 @@ export function Sparkline({
|
|||||||
return { path, areaPath, lastX: lx, lastY: ly };
|
return { path, areaPath, lastX: lx, lastY: ly };
|
||||||
}, [data, width, height, strokeWidth]);
|
}, [data, width, height, strokeWidth]);
|
||||||
|
|
||||||
const gradId = useMemo(
|
// useId() is SSR-stable; using Math.random() here would mismatch on hydration.
|
||||||
() => `bl-spark-${Math.random().toString(36).slice(2, 8)}`,
|
const reactId = useId();
|
||||||
[],
|
const gradId = `bl-spark-${reactId.replace(/[^a-zA-Z0-9_-]/g, '')}`;
|
||||||
);
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<svg
|
<svg
|
||||||
@ -100,7 +97,7 @@ export function Sparkline({
|
|||||||
strokeLinecap="round"
|
strokeLinecap="round"
|
||||||
strokeLinejoin="round"
|
strokeLinejoin="round"
|
||||||
/>
|
/>
|
||||||
{showLastPoint && data.length > 0 && (
|
{showLastPoint && data.length >= 2 && (
|
||||||
<circle
|
<circle
|
||||||
cx={lastX}
|
cx={lastX}
|
||||||
cy={lastY}
|
cy={lastY}
|
||||||
|
|||||||
@ -23,6 +23,25 @@ describe('Sparkline', () => {
|
|||||||
const path = screen.getByTestId('bl-sparkline').querySelector('path');
|
const path = screen.getByTestId('bl-sparkline').querySelector('path');
|
||||||
expect(path?.getAttribute('d')).toBe('');
|
expect(path?.getAttribute('d')).toBe('');
|
||||||
});
|
});
|
||||||
|
it('derives gradient id from React useId, not Math.random (SSR-safe)', () => {
|
||||||
|
// Regression: previously used Math.random() which caused Next.js
|
||||||
|
// hydration mismatches. React's useId emits ids shaped like ":r3:" /
|
||||||
|
// "_r_3_" — deterministic per render-position. We assert the id
|
||||||
|
// matches that shape (only the React-id chars), with no 6-hex random
|
||||||
|
// suffix.
|
||||||
|
const { container } = render(<Sparkline data={[1, 2, 3]} />);
|
||||||
|
const id = container.querySelector('linearGradient')?.getAttribute('id') ?? '';
|
||||||
|
expect(id).toMatch(/^bl-spark-[a-zA-Z0-9_-]+$/);
|
||||||
|
// Math.random().toString(36).slice(2, 8) always produced exactly 6
|
||||||
|
// chars after `bl-spark-`. useId produces something different.
|
||||||
|
const suffix = id.replace(/^bl-spark-/, '');
|
||||||
|
expect(/^[0-9a-f]{6}$/.test(suffix)).toBe(false);
|
||||||
|
});
|
||||||
|
it('omits the last-point circle when given a single datum', () => {
|
||||||
|
render(<Sparkline data={[5]} />);
|
||||||
|
// Single-point series has no meaningful trend, so no stale (0,0) circle.
|
||||||
|
expect(screen.getByTestId('bl-sparkline').querySelector('circle')).toBeNull();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('BarSparkline', () => {
|
describe('BarSparkline', () => {
|
||||||
@ -61,15 +80,11 @@ describe('KpiCard', () => {
|
|||||||
it('goodWhen=lower inverts the success/danger interpretation', () => {
|
it('goodWhen=lower inverts the success/danger interpretation', () => {
|
||||||
// For latency: a negative delta (faster) is good.
|
// For latency: a negative delta (faster) is good.
|
||||||
const { rerender } = render(
|
const { rerender } = render(
|
||||||
<KpiCard label="latency" value="120ms" deltaPercent={-10} goodWhen="lower" />,
|
<KpiCard label="latency" value="120ms" deltaPercent={-10} goodWhen="lower" />
|
||||||
);
|
);
|
||||||
expect(
|
expect(screen.getByTestId('bl-kpi-delta').getAttribute('data-good')).toBe('true');
|
||||||
screen.getByTestId('bl-kpi-delta').getAttribute('data-good'),
|
|
||||||
).toBe('true');
|
|
||||||
rerender(<KpiCard label="latency" value="120ms" deltaPercent={5} goodWhen="lower" />);
|
rerender(<KpiCard label="latency" value="120ms" deltaPercent={5} goodWhen="lower" />);
|
||||||
expect(
|
expect(screen.getByTestId('bl-kpi-delta').getAttribute('data-good')).toBe('false');
|
||||||
screen.getByTestId('bl-kpi-delta').getAttribute('data-good'),
|
|
||||||
).toBe('false');
|
|
||||||
});
|
});
|
||||||
it('renders sparkline when trend provided', () => {
|
it('renders sparkline when trend provided', () => {
|
||||||
render(<KpiCard label="X" value={1} trend={[1, 2, 3, 4]} />);
|
render(<KpiCard label="X" value={1} trend={[1, 2, 3, 4]} />);
|
||||||
@ -85,11 +100,18 @@ describe('ProgressRing', () => {
|
|||||||
rerender(<ProgressRing value={-1} />);
|
rerender(<ProgressRing value={-1} />);
|
||||||
expect(screen.getByTestId('bl-progress-ring').getAttribute('aria-label')).toBe('0 percent');
|
expect(screen.getByTestId('bl-progress-ring').getAttribute('aria-label')).toBe('0 percent');
|
||||||
});
|
});
|
||||||
|
it('treats NaN / non-finite value as 0 percent (no `NaN percent` aria-label)', () => {
|
||||||
|
render(<ProgressRing value={Number.NaN} />);
|
||||||
|
expect(screen.getByTestId('bl-progress-ring').getAttribute('aria-label')).toBe('0 percent');
|
||||||
|
cleanup();
|
||||||
|
render(<ProgressRing value={Number.POSITIVE_INFINITY} />);
|
||||||
|
expect(screen.getByTestId('bl-progress-ring').getAttribute('aria-label')).toBe('0 percent');
|
||||||
|
});
|
||||||
it('renders inner content slot', () => {
|
it('renders inner content slot', () => {
|
||||||
render(
|
render(
|
||||||
<ProgressRing value={0.5}>
|
<ProgressRing value={0.5}>
|
||||||
<span data-testid="center">50%</span>
|
<span data-testid="center">50%</span>
|
||||||
</ProgressRing>,
|
</ProgressRing>
|
||||||
);
|
);
|
||||||
expect(screen.getByTestId('center').textContent).toBe('50%');
|
expect(screen.getByTestId('center').textContent).toBe('50%');
|
||||||
});
|
});
|
||||||
@ -103,7 +125,14 @@ describe('Heatmap', () => {
|
|||||||
expect(screen.getAllByTestId(/bl-heatmap-cell-/).length).toBe(14);
|
expect(screen.getAllByTestId(/bl-heatmap-cell-/).length).toBe(14);
|
||||||
});
|
});
|
||||||
it('zero-value cells render with empty intensity', () => {
|
it('zero-value cells render with empty intensity', () => {
|
||||||
render(<Heatmap cells={[{ date: 'a', value: 0 }, { date: 'b', value: 5 }]} />);
|
render(
|
||||||
|
<Heatmap
|
||||||
|
cells={[
|
||||||
|
{ date: 'a', value: 0 },
|
||||||
|
{ date: 'b', value: 5 },
|
||||||
|
]}
|
||||||
|
/>
|
||||||
|
);
|
||||||
const a = screen.getByTestId('bl-heatmap-cell-0');
|
const a = screen.getByTestId('bl-heatmap-cell-0');
|
||||||
const b = screen.getByTestId('bl-heatmap-cell-1');
|
const b = screen.getByTestId('bl-heatmap-cell-1');
|
||||||
expect(a.getAttribute('data-intensity')).toBe('0.00');
|
expect(a.getAttribute('data-intensity')).toBe('0.00');
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user