test(tz): refuse a date formatter that does not name its calendar

A formatter with no timeZone follows the browser, which is a different
question from the one this app answers. Four surfaces had drifted apart that
way before anyone noticed, and one of them printed a shared report a day off
from the page it came from.

The admin host chart went with them rather than onto the allowlist. Server
samples are real instants, and an operator reading their own instance should
not find a spike at 14:20 here and 15:20 two screens away.

Two deliberate limits, both written into the test. It does not match
toLocaleString, which in this tree is overwhelmingly a number formatter, and
burying four real hits under every thousands separator is how a guard gets
silenced. And it cannot tell a Date built from calendar arithmetic — a month
label, a weekday header, no instant behind it — from one parsed out of stored
data, so those four sites carry a written reason instead. The value is the
ratchet: a new bare formatter has to be argued for before it ships.
This commit is contained in:
Marc-André Bombeck committed 2026-07-27 00:18:22 +02:00
1 parent f1773cf602
commit 8e9da490a9
2 files changed
+163

No files matched your search

@@ -0,0 +1,157 @@
import { readFileSync } from "node:fs";
import { execFileSync } from "node:child_process";
import { resolve } from "node:path";
import { describe, expect, it } from "vitest";
/**
* A formatter that renders an instant must say which calendar it renders in.
*
* `new Intl.DateTimeFormat(locale, {…})` and `date.toLocaleDateString(…)` fall
* back to the BROWSER's timezone when none is given. That is a different
* question from the one this app answers: a reading belongs to the day it was
* where the person lives, not where their laptop happens to be. A reporter hit
* exactly this (#490), and the sweep behind it found four separate surfaces
* that had drifted apart — one of them printed a shared report a day off from
* the page it was downloaded from.
*
* The check is a source-shape check, and it is deliberately coarse in one
* direction: it cannot tell a Date built from calendar arithmetic (a month
* label, a weekday header — no instant involved, no timezone question) from a
* Date parsed out of stored data. So those legitimately-bare sites live on the
* allowlist with a reason, and the guard's real job is the ratchet: a NEW bare
* formatter has to be argued for in writing before it can ship.
*
* `.toLocaleString(` is deliberately NOT matched. It is overwhelmingly a
* number formatter in this tree, and flagging thousands separators would bury
* the four real hits in noise until someone silenced the whole guard.
*/
const REPO_ROOT = resolve(__dirname, "../..");
/**
* Bare formatter sites that are correct as they stand. Keep this short: it is
* an inventory of exceptions, not a parking space. Every entry says why.
*/
const ALLOWLIST: ReadonlyArray<{ file: string; why: string }> = [
{
file: "src/components/ui/calendar.tsx",
why: "A `data-day` attribute on a calendar cell, derived from the grid's own arithmetic. It identifies a cell, it does not render a recorded moment.",
},
{
file: "src/components/cycle/cycle-calendar.tsx",
why: "Weekday headers walked from a fixed local date, and a month label built from a month anchor. Both are calendar arithmetic with no instant behind them.",
},
{
file: "src/lib/intl/formatter-cache.ts",
why: "The shared cache itself. Its options come from the caller, so the timezone is the caller's to supply and the guard should be asking them, not this.",
},
];
const FORMATTER_PATTERNS = [
/new Intl\.DateTimeFormat\s*\(/g,
/\.toLocale(?:Date|Time)String\s*\(/g,
];
/** The argument list starting at the `(` at or after `from`. */
function argumentList(source: string, from: number): string {
const open = source.indexOf("(", from);
if (open === -1) return "";
let depth = 0;
for (let i = open; i < source.length; i += 1) {
if (source[i] === "(") depth += 1;
else if (source[i] === ")") {
depth -= 1;
if (depth === 0) return source.slice(open, i + 1);
}
}
return source.slice(open);
}
function bareFormatterSites(): Array<{ file: string; line: number }> {
const listed = execFileSync(
"grep",
[
"-rlE",
"new Intl\\.DateTimeFormat|\\.toLocale(Date|Time)String\\(",
"src",
"--include=*.ts",
"--include=*.tsx",
],
{ cwd: REPO_ROOT, encoding: "utf8" },
)
.trim()
.split("\n")
.filter(Boolean)
// Tests pin instants on purpose and are not user-facing surfaces.
.filter((f) => !f.includes("__tests__"));
const hits: Array<{ file: string; line: number }> = [];
for (const file of listed) {
const source = readFileSync(resolve(REPO_ROOT, file), "utf8");
for (const pattern of FORMATTER_PATTERNS) {
pattern.lastIndex = 0;
for (let m = pattern.exec(source); m !== null; m = pattern.exec(source)) {
const args = argumentList(source, m.index + m[0].length - 1);
if (args.includes("timeZone")) continue;
hits.push({
file,
line: source.slice(0, m.index).split("\n").length,
});
}
}
}
return hits;
}
describe("date formatters name the calendar they render in", () => {
const hits = bareFormatterSites();
it("finds formatter sites at all (the guard must not pass vacuously)", () => {
// If this drops to zero the grep has stopped matching and every assertion
// below became free.
const all = execFileSync(
"grep",
[
"-rlE",
"new Intl\\.DateTimeFormat|\\.toLocale(Date|Time)String\\(",
"src",
"--include=*.ts",
"--include=*.tsx",
],
{ cwd: REPO_ROOT, encoding: "utf8" },
)
.trim()
.split("\n")
.filter(Boolean);
expect(all.length).toBeGreaterThan(5);
});
it("has no unexplained formatter without an explicit timeZone", () => {
const unexplained = hits.filter(
(h) => !ALLOWLIST.some((a) => a.file === h.file),
);
if (unexplained.length > 0) {
throw new Error(
`${unexplained.length} formatter call(s) render a date without naming a timezone:\n\n` +
unexplained.map((h) => ` ${h.file}:${h.line}`).join("\n") +
"\n\nWithout `timeZone` these follow the browser, not the profile. Pass the " +
"profile timezone (`readStoredTimezone() || DEFAULT_TIMEZONE` on the client, " +
"`resolveUserTimezone(userId)` on the server), or use the shared helpers in " +
"@/lib/format. If the value is calendar arithmetic rather than a recorded " +
"instant, add the file to ALLOWLIST in this test with the reason.",
);
}
});
it("carries no allowlist entry that no longer has a bare formatter", () => {
// A stale exception is worse than none: it documents a decision about code
// that has moved on, and it quietly covers whatever lands in that file next.
for (const entry of ALLOWLIST) {
expect(
hits.some((h) => h.file === entry.file),
`stale allowlist entry — ${entry.file} no longer has a bare formatter`,
).toBe(true);
}
});
});
@@ -41,6 +41,7 @@ import { queryKeys } from "@/lib/query-keys";
import { Skeleton } from "@/components/ui/skeleton";
import { SettingsCard } from "@/components/settings/settings-card";
import { apiGet } from "@/lib/api/api-fetch";
import { readStoredTimezone } from "@/lib/timezone-mirror";
interface HostMetricsApiSample {
capturedAt: string;
@@ -94,10 +95,15 @@ export function buildChartRows(samples: HostMetricsApiSample[]): {
if (total > peakDiskBps) peakDiskBps = total;
}
// The profile's timezone, not the browser's. These are real instants (host
// samples), and an operator reading their own instance should see the same
// clock here as on every other timestamp in the admin area — a spike at
// "14:20" must not be 14:20 here and 15:20 two screens away.
const formatter = new Intl.DateTimeFormat(undefined, {
hour: "2-digit",
minute: "2-digit",
hour12: false,
timeZone: readStoredTimezone() || undefined,
});
let hasDiskData = false;