bbovenzi commented on code in PR #72350:
URL: https://github.com/apache/airflow/pull/72350#discussion_r3915438227


##########
airflow-core/src/airflow/ui/src/utils/datetimeUtils.ts:
##########
@@ -29,71 +30,199 @@ export const DATE_FORMAT = "YYYY-MM-DD";
 export const DEFAULT_DATETIME_FORMAT = `${DATE_FORMAT} HH:mm:ss`;
 export const DEFAULT_DATETIME_FORMAT_WITH_TZ = `${DEFAULT_DATETIME_FORMAT} z`;
 
-export const renderDuration = (
-  durationSeconds: dayjsDuration.Duration | number | null | undefined,
-  withMilliseconds: boolean = true,
-): string | undefined => {
-  if (durationSeconds === null || durationSeconds === undefined) {
-    return undefined;
+const DEFAULT_LOCALE = "en";
+const SECONDS_PER_MINUTE = 60;
+const SECONDS_PER_HOUR = 3600;
+const SECONDS_PER_DAY = 86_400;
+
+type DurationUnit = "day" | "hour" | "millisecond" | "minute" | "second";
+
+type DurationPart = { fractionDigits?: number; unit: DurationUnit; value: 
number };
+
+/** `narrow` ("1h 2m") suits dense tables and charts; `long` ("1 hour, 2 
minutes") suits prose. */
+type DurationStyle = "long" | "narrow";
+
+// Durations render in every table row and chart tick callback, and Intl 
formatters are costly to
+// construct, so instances are reused. A stored language Intl rejects must not 
blank out every
+// duration in the UI, hence the fallback instead of letting the RangeError 
escape.
+const unitFormatters = new Map<string, Intl.NumberFormat>();
+
+const getUnitFormatter = (locale: string, style: DurationStyle, part: 
DurationPart): Intl.NumberFormat => {
+  const { fractionDigits = 0, unit } = part;
+  const key = `${locale}|${unit}|${fractionDigits}|${style}`;
+  const cached = unitFormatters.get(key);
+
+  if (cached !== undefined) {
+    return cached;
   }
 
-  // Handle floating point milliseconds
-  const duration = dayjs.isDuration(durationSeconds)
-    ? dayjs.duration(Math.round(durationSeconds.asMilliseconds()))
-    : dayjs.duration(Number(durationSeconds.toFixed(3)), "seconds");
+  const options: Intl.NumberFormatOptions = {
+    maximumFractionDigits: fractionDigits,
+    style: "unit",
+    unit,
+    unitDisplay: style,
+  };
+  let formatter: Intl.NumberFormat;
 
-  if (duration.asMilliseconds() < 1) {
-    return undefined;
+  try {
+    formatter = new Intl.NumberFormat(locale, options);
+  } catch {
+    formatter = new Intl.NumberFormat(DEFAULT_LOCALE, options);
   }
 
-  // If under 60 seconds, render milliseconds
-  if (duration.asSeconds() < 60 && duration.milliseconds() > 0 && 
withMilliseconds) {
-    return duration.format("HH:mm:ss.SSS");
+  unitFormatters.set(key, formatter);
+
+  return formatter;
+};
+
+const listFormatters = new Map<string, Intl.ListFormat>();
+
+const getListFormatter = (locale: string, style: DurationStyle): 
Intl.ListFormat => {
+  const key = `${locale}|${style}`;
+  const cached = listFormatters.get(key);
+
+  if (cached !== undefined) {
+    return cached;
   }
 
-  // If under 1 day, render as HH:mm:ss otherwise include the number of days
-  return duration.asSeconds() < 86_400 ? duration.format("HH:mm:ss") : 
duration.format("D[d]HH:mm:ss");
+  const options: Intl.ListFormatOptions = { style, type: "unit" };
+  let formatter: Intl.ListFormat;
+
+  try {
+    formatter = new Intl.ListFormat(locale, options);
+  } catch {
+    formatter = new Intl.ListFormat(DEFAULT_LOCALE, options);
+  }
+
+  listFormatters.set(key, formatter);
+
+  return formatter;
+};
+
+// Unit names, decimal separators, plural forms and the joiner all come from 
CLDR, so "1h 2m" is
+// "1 ч 2 мин" in ru. This reproduces Intl.DurationFormat's narrow style 
exactly (verified across
+// every locale we ship) without requiring it: that API needs Node 23+, above 
this package's
+// engines floor, and Node 23 was never an LTS line. Exact wording also varies 
by the runtime's ICU
+// version, so nothing may depend on a specific CLDR string.
+const formatParts = (parts: Array<DurationPart>, locale: string, style: 
DurationStyle): string => {
+  const formatted = parts.map((part) => getUnitFormatter(locale, style, 
part).format(part.value));
+
+  return formatted.length > 1 ? getListFormatter(locale, 
style).format(formatted) : (formatted[0] ?? "");
 };
 
-// dayjs humanizes a missing or non-finite input as "a few seconds", so 
callers with no duration
-// to name get undefined instead of a made-up one.
-export const humanizeSeconds = (seconds: number | null | undefined): string | 
undefined =>
-  typeof seconds === "number" && Number.isFinite(seconds)
-    ? dayjs.duration(seconds, "seconds").humanize()
-    : undefined;
+// Durations carry roughly three significant digits at every magnitude, so a 
83ms task and a
+// three-day backfill are both legible without decoding zero-padded clock 
groups. Rounding at a
+// band's precision can spill into the next band (59.96s is a minute, not 
"60.0s"), hence the
+// recursion on the promoted value. Callers needing the unrounded number 
should surface it separately.
+const getDurationParts = (seconds: number): Array<DurationPart> => {
+  if (seconds === 0) {
+    return [{ unit: "second", value: 0 }];
+  }
+
+  if (seconds < 1) {
+    const milliseconds = Math.round(seconds * 1000);
+
+    return milliseconds < 1000 ? [{ unit: "millisecond", value: milliseconds 
}] : getDurationParts(1);
+  }
+
+  if (seconds < SECONDS_PER_MINUTE) {
+    // Two decimals under 10s, one above, keeps three significant digits 
either way.
+    const fractionDigits = seconds < 10 ? 2 : 1;
+    const rounded = Number(seconds.toFixed(fractionDigits));
+
+    return rounded < SECONDS_PER_MINUTE
+      ? [{ fractionDigits, unit: "second", value: rounded }]
+      : getDurationParts(SECONDS_PER_MINUTE);
+  }
+
+  if (seconds < SECONDS_PER_HOUR) {
+    const minutes = Math.floor(seconds / SECONDS_PER_MINUTE);
+    const remainingSeconds = Math.round(seconds - minutes * 
SECONDS_PER_MINUTE);
+
+    if (remainingSeconds === SECONDS_PER_MINUTE) {
+      return getDurationParts((minutes + 1) * SECONDS_PER_MINUTE);
+    }
 
-// Chart axes need whole units at a glance; HH:mm:ss forces the reader to 
decode
-// every tick to work out the magnitude.
-export const renderCompactDuration = (durationSeconds: number): string => {
-  if (!Number.isFinite(durationSeconds) || durationSeconds <= 0) {
-    return "0s";
+    return remainingSeconds > 0
+      ? [
+          { unit: "minute", value: minutes },
+          { unit: "second", value: remainingSeconds },
+        ]
+      : [{ unit: "minute", value: minutes }];
   }
 
-  if (durationSeconds < 1) {
-    return `${Math.round(durationSeconds * 1000)}ms`;
+  if (seconds < SECONDS_PER_DAY) {
+    const hours = Math.floor(seconds / SECONDS_PER_HOUR);
+    const remainingMinutes = Math.round((seconds - hours * SECONDS_PER_HOUR) / 
SECONDS_PER_MINUTE);
+
+    if (remainingMinutes === SECONDS_PER_MINUTE) {

Review Comment:
   Fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to