Skip to content

Commit 428084b

Browse files
committed
PMM-15326: Put the OM plugin on date-fns and plugin-react
Two review notes on #5817, both about using what the monorepo already has. `vitest.config.ts` used `@vitejs/plugin-react-swc`. `main` moved to `@vitejs/plugin-react` and vite itself now prints the recommendation on every run, so this package follows. Only this package - the rest of the tree is not this PR's to move. `format.ts` hand-rolled parsing and date arithmetic that date-fns does, and date-fns is already a dependency of `apps/pmm`. Parsing, validity and differences now come from `parseISO`, `isValid`, `differenceInMilliseconds` and `format`. The compact duration formatter stays, and is renamed `formatCompactDuration` to say why. Its callers are table cells and chips that need `2d 3h`; date-fns' `formatDuration` emits `2 days 3 hours`, and the short form needs a custom `locale.formatDistance` - more code than the arithmetic it replaces. The rename also ends a collision with date-fns' export of the same name, so a file can now import both. Two behaviours worth naming. `formatTimestamp` moves from `toLocaleString()` to a fixed `yyyy-MM-dd HH:mm:ss` in local time: run timestamps are read down a column, and a format that varies by locale is not a column anyone can scan. `formatAge` deliberately does not use `formatDistanceToNowStrict`, which rounds to one unit - these columns have to distinguish `1h 12m` from `1h 58m`. Signed-off-by: Pawel Lebioda <pawel.lebioda@percona.com>
1 parent 4149c92 commit 428084b

12 files changed

Lines changed: 145 additions & 66 deletions

File tree

ui/packages/plugins/om/package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
"@testing-library/react": "^16.3.0",
2525
"@testing-library/user-event": "^14.6.1",
2626
"@types/react": "^19.1.0",
27-
"@vitejs/plugin-react-swc": "^4.3.1",
27+
"@vitejs/plugin-react": "^6.1.0",
2828
"jsdom": "^29.1.1",
2929
"material-react-table": "^3.2.1",
3030
"notistack": "^3.0.0",
@@ -45,5 +45,8 @@
4545
"notistack": "^3.0.0",
4646
"react": "^19.0.0",
4747
"react-router-dom": "^7.14.0"
48+
},
49+
"dependencies": {
50+
"date-fns": "4.1.0"
4851
}
4952
}

ui/packages/plugins/om/src/HostsPage.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import {
4343
} from './constants';
4444
import { OmHeader } from './components/OmHeader';
4545
import { Unavailable } from './components/Unavailable';
46-
import { formatDuration } from './format';
46+
import { formatCompactDuration } from './format';
4747
import { ageSeconds, isFailing, toHostRows } from './inventory';
4848
import {
4949
useForgetHost,
@@ -224,18 +224,18 @@ function useColumns(): MRT_ColumnDef<OmHostRow>[] {
224224
}
225225
const since = ageSeconds(original.freshness.failing_since);
226226
return since == null ? (
227-
<>{formatDuration(age)} ago</>
227+
<>{formatCompactDuration(age)} ago</>
228228
) : (
229229
<Tooltip
230-
title={`${original.freshness.last_error ?? 'The last probe failed.'} Failing for ${formatDuration(
230+
title={`${original.freshness.last_error ?? 'The last probe failed.'} Failing for ${formatCompactDuration(
231231
since
232232
)}, ${original.freshness.consecutive_failures} attempts.`}
233233
>
234234
<Box
235235
component="span"
236236
sx={{ color: 'error.main', cursor: 'help' }}
237237
>
238-
{formatDuration(age)} ago (failing)
238+
{formatCompactDuration(age)} ago (failing)
239239
</Box>
240240
</Tooltip>
241241
);

ui/packages/plugins/om/src/InventoryPage.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import { RunStatusBadge } from './components/HealthBadge';
4141
import { RunEntities } from './components/RunEntities';
4242
import { OmHeader } from './components/OmHeader';
4343
import {
44-
formatDuration,
44+
formatCompactDuration,
4545
formatRunDuration,
4646
formatTimestamp,
4747
runDurationSeconds,
@@ -245,8 +245,8 @@ function LastRun({ run }: { run: OmInventoryRun | undefined }) {
245245
<RunStatusBadge status={run.status} />
246246
<Typography variant="body2" color="text.secondary">
247247
{active
248-
? `started ${age == null ? 'just now' : `${formatDuration(age)} ago`}`
249-
: `${age == null ? '' : `${formatDuration(age)} ago`}, took ${
248+
? `started ${age == null ? 'just now' : `${formatCompactDuration(age)} ago`}`
249+
: `${age == null ? '' : `${formatCompactDuration(age)} ago`}, took ${
250250
formatRunDuration(run.start_time, run.end_time) || '—'
251251
}`}
252252
</Typography>

ui/packages/plugins/om/src/ServicesPage.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import {
4646
missingRowReason,
4747
type OmEstateStatus,
4848
} from './inventory';
49-
import { formatDuration } from './format';
49+
import { formatCompactDuration } from './format';
5050
import { ProbeValue } from './components/ProbeValue';
5151
import type { OmInventoryService, OmServiceInventoryRow } from './types';
5252

@@ -181,7 +181,7 @@ function useColumns(
181181
return age == null ? (
182182
<Unavailable reason="probe_never_succeeded" />
183183
) : (
184-
<>{formatDuration(age)} ago</>
184+
<>{formatCompactDuration(age)} ago</>
185185
);
186186
},
187187
},
@@ -372,7 +372,7 @@ function ProbeStatus({ inventory }: { inventory: OmInventoryService }) {
372372
}
373373
>
374374
<Box component="span" sx={{ color: 'error.main', cursor: 'help' }}>
375-
failing {formatDuration(since)}
375+
failing {formatCompactDuration(since)}
376376
{inventory.freshness.consecutive_failures > 0
377377
? ` (${inventory.freshness.consecutive_failures}x)`
378378
: ''}

ui/packages/plugins/om/src/components/Metric.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* along with this program. If not, see <https://www.gnu.org/licenses/>.
1616
*/
1717

18-
import { formatDuration } from '../format';
18+
import { formatCompactDuration } from '../format';
1919
import { Unavailable } from './Unavailable';
2020

2121
/** Reading that means "not measured" in the two percentage columns. */
@@ -45,5 +45,5 @@ export function Duration({ value }: { value: number | null | undefined }) {
4545
if (value === null || value === undefined) {
4646
return <Unavailable reason="not_applicable" />;
4747
}
48-
return <>{formatDuration(value) || '0s'}</>;
48+
return <>{formatCompactDuration(value) || '0s'}</>;
4949
}

ui/packages/plugins/om/src/components/ProbeValue.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import {
2121
missingRowReason,
2222
type OmEstateStatus,
2323
} from '../inventory';
24-
import { formatDuration } from '../format';
24+
import { formatCompactDuration } from '../format';
2525
import { Unavailable } from './Unavailable';
2626
import type { OmInventoryService } from '../types';
2727

@@ -86,10 +86,10 @@ export function ProbeValue({
8686
const detail = [
8787
age == null
8888
? 'This service has never answered a probe.'
89-
: `Last collected ${formatDuration(age)} ago.`,
89+
: `Last collected ${formatCompactDuration(age)} ago.`,
9090
since == null
9191
? null
92-
: `Failing for ${formatDuration(since)} (${inventory.freshness.consecutive_failures} attempts).`,
92+
: `Failing for ${formatCompactDuration(since)} (${inventory.freshness.consecutive_failures} attempts).`,
9393
inventory.freshness.last_error,
9494
]
9595
.filter(Boolean)
@@ -117,7 +117,7 @@ export function ProbeValue({
117117
component="span"
118118
sx={{ color: 'warning.main', ml: 0.5, whiteSpace: 'nowrap' }}
119119
>
120-
{age == null ? '(stale)' : `(${formatDuration(age)} old)`}
120+
{age == null ? '(stale)' : `(${formatCompactDuration(age)} old)`}
121121
</Box>
122122
</Box>
123123
</Tooltip>

ui/packages/plugins/om/src/components/RunEntities.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {
3030
Typography,
3131
} from '@mui/material';
3232
import { useOmInventoryRun } from '../inventoryHooks';
33-
import { formatDuration } from '../format';
33+
import { formatCompactDuration } from '../format';
3434
import { Unavailable } from './Unavailable';
3535
import type { OmExecutorResolution, OmInventoryRun } from '../types';
3636

@@ -48,15 +48,15 @@ const RESOLUTION_LABEL: Record<OmExecutorResolution, string> = {
4848
/**
4949
* A sub-second host time still deserves a number.
5050
*
51-
* `formatDuration` floors to whole seconds, which is right for an oplog window of days
51+
* `formatCompactDuration` floors to whole seconds, which is right for an oplog window of days
5252
* and wrong here: a host that failed in 0.28s would read as `0s`, hiding that it
5353
* failed instantly rather than after a wait.
5454
*/
5555
function formatHostDuration(seconds: number): string {
5656
if (seconds < 10) {
5757
return `${seconds.toFixed(2)}s`;
5858
}
59-
return formatDuration(seconds) || `${Math.round(seconds)}s`;
59+
return formatCompactDuration(seconds) || `${Math.round(seconds)}s`;
6060
}
6161

6262
/**

ui/packages/plugins/om/src/format.ts

Lines changed: 64 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,51 @@
1515
* along with this program. If not, see <https://www.gnu.org/licenses/>.
1616
*/
1717

18-
/** Format a duration in seconds as a compact `2d 3h` / `4m 12s` string. */
19-
export function formatDuration(seconds: number | null | undefined): string {
18+
/**
19+
* Formatting helpers, on `date-fns` where `date-fns` has the answer.
20+
*
21+
* It is the monorepo's date library and `apps/pmm` already uses it, so parsing,
22+
* validity and differences come from there rather than from arithmetic kept here.
23+
*
24+
* `formatCompactDuration` is the exception, and deliberately not date-fns'
25+
* `formatCompactDuration`. Every caller is a table cell or a chip, where the string has to be
26+
* `2d 3h` rather than `2 days 3 hours`; date-fns emits words, and the short form needs
27+
* a custom `locale.formatDistance` - more code than the arithmetic it would replace. It
28+
* is named apart from date-fns' export of the same name so a file can import both.
29+
*/
30+
31+
import { differenceInMilliseconds, format, isValid, parseISO } from 'date-fns';
32+
33+
/** Placeholder for a value that has no usable timestamp behind it. */
34+
const NO_VALUE = '—';
35+
36+
/** How a timestamp is shown when it is shown absolutely. */
37+
const TIMESTAMP_FORMAT = 'yyyy-MM-dd HH:mm:ss';
38+
39+
/**
40+
* Parse a wire timestamp, or null when there is nothing usable.
41+
*
42+
* One place for the two failure modes every helper here shares: no value at all, and a
43+
* value that will not parse. `parseISO` answers an Invalid Date rather than throwing,
44+
* so `isValid` is what separates them.
45+
*/
46+
function parse(iso: string | null | undefined): Date | null {
47+
if (!iso) {
48+
return null;
49+
}
50+
const date = parseISO(iso);
51+
return isValid(date) ? date : null;
52+
}
53+
54+
/**
55+
* Format a duration in seconds as a compact `2d 3h` / `4m 12s` string.
56+
*
57+
* Two units, never more: these land in table cells beside other numbers, and
58+
* `2d 3h 7m 12s` reads as noise where `2d 3h` reads at a glance.
59+
*/
60+
export function formatCompactDuration(
61+
seconds: number | null | undefined
62+
): string {
2063
if (seconds == null || !Number.isFinite(seconds) || seconds < 0) {
2164
return '';
2265
}
@@ -38,7 +81,6 @@ export function formatDuration(seconds: number | null | undefined): string {
3881
parts.push(`${value}${suffix}`);
3982
left -= value * size;
4083
}
41-
// Two units is enough to read at a glance; `2d 3h` beats `2d 3h 7m 12s`.
4284
if (parts.length === 2) {
4385
break;
4486
}
@@ -48,34 +90,31 @@ export function formatDuration(seconds: number | null | undefined): string {
4890

4991
/** Absolute local timestamp, or an em-dash placeholder when there is none. */
5092
export function formatTimestamp(iso: string | null | undefined): string {
51-
if (!iso) {
52-
return '—';
53-
}
54-
const date = new Date(iso);
55-
if (Number.isNaN(date.getTime())) {
56-
return '—';
57-
}
58-
return date.toLocaleString();
93+
const date = parse(iso);
94+
return date ? format(date, TIMESTAMP_FORMAT) : NO_VALUE;
5995
}
6096

6197
/**
6298
* Age of a timestamp as `3m ago`, relative to `now`.
6399
*
100+
* Deliberately not `formatDistanceToNowStrict`: that rounds to a single unit and
101+
* answers "3 minutes ago", and these are columns where `1h 12m` and `1h 58m` have to
102+
* differ. The difference comes from date-fns; the rendering is the compact form above.
103+
*
64104
* `now` is injectable so tests do not depend on the clock.
65105
*/
66106
export function formatAge(
67107
iso: string | null | undefined,
68108
now: number = Date.now()
69109
): string {
70-
if (!iso) {
71-
return '—';
110+
const date = parse(iso);
111+
if (!date) {
112+
return NO_VALUE;
72113
}
73-
const then = new Date(iso).getTime();
74-
if (Number.isNaN(then)) {
75-
return '—';
76-
}
77-
const seconds = Math.max(0, (now - then) / 1000);
78-
return `${formatDuration(seconds) || '0s'} ago`;
114+
// Clamped rather than signed: a host whose clock runs ahead should read as "just
115+
// collected", not as a negative age.
116+
const seconds = Math.max(0, differenceInMilliseconds(now, date) / 1000);
117+
return `${formatCompactDuration(seconds) || '0s'} ago`;
79118
}
80119

81120
/**
@@ -88,15 +127,13 @@ export function runDurationSeconds(
88127
startedAt: string,
89128
finishedAt: string | null | undefined
90129
): number | null {
91-
if (!finishedAt) {
92-
return null;
93-
}
94-
const start = new Date(startedAt).getTime();
95-
const end = new Date(finishedAt).getTime();
96-
if (Number.isNaN(start) || Number.isNaN(end) || end < start) {
130+
const start = parse(startedAt);
131+
const end = parse(finishedAt);
132+
if (!start || !end) {
97133
return null;
98134
}
99-
return (end - start) / 1000;
135+
const elapsed = differenceInMilliseconds(end, start);
136+
return elapsed < 0 ? null : elapsed / 1000;
100137
}
101138

102139
/** Wall-clock length of a run; empty while it is still going. */
@@ -108,5 +145,5 @@ export function formatRunDuration(
108145
if (seconds === null) {
109146
return '';
110147
}
111-
return formatDuration(seconds) || '0s';
148+
return formatCompactDuration(seconds) || '0s';
112149
}

ui/packages/plugins/om/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ export {
8383
export type { OmEstateStatus } from './inventory';
8484
export {
8585
formatAge,
86-
formatDuration,
86+
formatCompactDuration,
8787
formatRunDuration,
8888
runDurationSeconds,
8989
formatTimestamp,

ui/packages/plugins/om/tests/format.test.ts

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,35 +18,35 @@
1818
import { describe, expect, it } from 'vitest';
1919
import {
2020
formatAge,
21-
formatDuration,
21+
formatCompactDuration,
2222
formatRunDuration,
2323
formatTimestamp,
2424
} from '../src/format';
2525

26-
describe('formatDuration', () => {
26+
describe('formatCompactDuration', () => {
2727
it('renders sub-minute values in seconds', () => {
28-
expect(formatDuration(0)).toBe('0s');
29-
expect(formatDuration(47)).toBe('47s');
28+
expect(formatCompactDuration(0)).toBe('0s');
29+
expect(formatCompactDuration(47)).toBe('47s');
3030
});
3131

3232
it('stops at two units so a value stays glanceable', () => {
33-
expect(formatDuration(47323)).toBe('13h 8m');
34-
expect(formatDuration(90)).toBe('1m 30s');
35-
expect(formatDuration(2 * 86400 + 3 * 3600 + 7 * 60)).toBe('2d 3h');
33+
expect(formatCompactDuration(47323)).toBe('13h 8m');
34+
expect(formatCompactDuration(90)).toBe('1m 30s');
35+
expect(formatCompactDuration(2 * 86400 + 3 * 3600 + 7 * 60)).toBe('2d 3h');
3636
});
3737

3838
it('skips units that are zero rather than padding them', () => {
39-
expect(formatDuration(86400)).toBe('1d');
40-
expect(formatDuration(3600)).toBe('1h');
39+
expect(formatCompactDuration(86400)).toBe('1d');
40+
expect(formatCompactDuration(3600)).toBe('1h');
4141
});
4242

4343
// A null here means "not observed", and the caller renders <Unavailable/>.
4444
// Returning '0s' would turn an absent observation into a real measurement.
4545
it('returns empty for values that are not measurements', () => {
46-
expect(formatDuration(null)).toBe('');
47-
expect(formatDuration(undefined)).toBe('');
48-
expect(formatDuration(-1)).toBe('');
49-
expect(formatDuration(Number.NaN)).toBe('');
46+
expect(formatCompactDuration(null)).toBe('');
47+
expect(formatCompactDuration(undefined)).toBe('');
48+
expect(formatCompactDuration(-1)).toBe('');
49+
expect(formatCompactDuration(Number.NaN)).toBe('');
5050
});
5151
});
5252

@@ -72,6 +72,20 @@ describe('formatTimestamp', () => {
7272
expect(formatTimestamp(null)).toBe('—');
7373
expect(formatTimestamp('nonsense')).toBe('—');
7474
});
75+
76+
// A fixed pattern rather than toLocaleString: run timestamps are compared against
77+
// one another down a column, and a format that varies with the reader's locale is
78+
// not a column you can scan. The rendering is local time, so the assertion goes
79+
// through the same conversion rather than hard-coding a zone.
80+
it('renders a parsable timestamp in local time', () => {
81+
const iso = '2026-08-07T09:58:48Z';
82+
const local = new Date(iso);
83+
const pad = (value: number) => String(value).padStart(2, '0');
84+
expect(formatTimestamp(iso)).toBe(
85+
`${local.getFullYear()}-${pad(local.getMonth() + 1)}-${pad(local.getDate())} ` +
86+
`${pad(local.getHours())}:${pad(local.getMinutes())}:${pad(local.getSeconds())}`
87+
);
88+
});
7589
});
7690

7791
describe('formatRunDuration', () => {

0 commit comments

Comments
 (0)