diff --git a/.changeset/7745-formatdate-options-style.md b/.changeset/7745-formatdate-options-style.md new file mode 100644 index 0000000000..4a9296ffbe --- /dev/null +++ b/.changeset/7745-formatdate-options-style.md @@ -0,0 +1,33 @@ +--- +'@object-ui/core': patch +--- + +`formatDate` reads `options.style` (objectui#7745). + +`DateDisplayOptions` is the one bag `formatDate` / `formatRelativeDate` / +`formatDateTime` share. `style` was added to it for `formatDateTime`'s `'compact'` +grid face (objectui#7443, PR #7621) and only `formatDateTime` read it, so on +`formatDate` the key was inert — and inert beside a POSITIONAL parameter of the same +name. `formatDate(v, undefined, { style: 'short', locale: 'en-US' })` rendered +`Jul 4, 2024`, the default face, with no diagnostic; it now renders `Jul 4, '24`. +This is the additive half of the maintainer's long-run ruling on objectui#7443: +both functions accepting `options.style`. + +**The precedence is pinned: the positional argument wins.** `options.style` is +consulted only when the positional slot is `undefined` (`??`, not `||`, so `''` +still counts as given). That is the only direction that is purely additive — it +fires exactly on the input that is a silent no-op today, so no call that renders a +face today renders a different one after. The reverse would let a key aimed at a +SIBLING function outrank an argument written for this call: the bag is shared, and +carrying `{ style: 'compact', locale }` built for `formatDateTime` into +`formatDate(v, 'short', bag)` must not cost that call its short face. + +**What changes for you.** Only `formatDate(value, undefined, { style: 'short' | 'relative' })` — +a call that silently rendered the default face before. Every call that passes the +style positionally, and every `formatDateTime` / `formatRelativeDate` call, renders +byte-identically to before. + +`formatRelativeDate` still does NOT read `style`; the ruling names `formatDate` +only. Its out-of-window fallback to `formatDate` strips the key so that the new read +cannot leak in through the delegation — which also keeps +`formatRelativeDate(v, { style: 'relative' })` from recursing. diff --git a/packages/core/src/utils/__tests__/date-display.optionsStyle-7745.test.ts b/packages/core/src/utils/__tests__/date-display.optionsStyle-7745.test.ts new file mode 100644 index 0000000000..84630551b0 --- /dev/null +++ b/packages/core/src/utils/__tests__/date-display.optionsStyle-7745.test.ts @@ -0,0 +1,188 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * objectui#7745 — `formatDate` reads `options.style`, and the precedence + * against its same-named positional parameter is PINNED. + * + * ── What was measured, at head `a617bb8d3` ────────────────────────────────── + * `DateDisplayOptions` is the ONE bag `formatDate` / `formatRelativeDate` / + * `formatDateTime` share. PR #7621 added `style?: string` to it and only + * `formatDateTime` read it, so on `formatDate` the key was inert — and inert + * next to a POSITIONAL parameter of the same name: + * + * | call | before | + * | -------------------------------------------------------- | ------------- | + * | `formatDate(v, undefined, { style: 'short', locale })` | `Jul 4, 2024` | + * | `formatDate(v, 'short', { locale })` | `Jul 4, '24` | + * + * One function, two spellings for one concept, one of them silently doing + * nothing — and the silent one is the spelling `formatDateTime` REQUIRES. The + * maintainer's ruling B on objectui#7443 (comment 5539935824) names the + * long-run shape: both functions accepting `options.style`, additive on + * `formatDate`. This file is that step's pin. + * + * ── The precedence, and why this direction ───────────────────────────────── + * **The positional argument wins**; `options.style` is consulted only when the + * positional slot is `undefined`. It is the only direction that is purely + * ADDITIVE: it fires exactly on the input that is a silent no-op today, so no + * call that renders a face today renders a different one after. The reverse + * would let a key aimed at a SIBLING function (the bag is shared, and + * `dueLike` / `t` are read by `formatRelativeDate` alone) outrank an argument + * the caller wrote for THIS call — objectui#7694's shape, and the silent + * override half of objectui#4272. `??` and not `||`, so `''` still counts as + * given and keeps rendering the default face. + * + * ── Directions, predicted in writing BEFORE the run ──────────────────────── + * Reverting the read (`style ?? options?.style` → `style`) + * RED: every case in "the options spelling is read at all" and in "the + * options spelling IS the positional spelling"; the precedence cases + * stay GREEN (positional still wins by absence of any competitor), and + * so does everything under "nothing that renders a face today moves". + * Inverting the precedence (`options?.style ?? style`) + * RED: only "the positional argument wins"; the additive cases stay + * GREEN. This is the pair that makes the two halves independently + * falsifiable — neither mutation can turn both red. + * Dropping the `absoluteFallbackOptions` strip in `formatRelativeDate` + * RED: "`formatRelativeDate` still does not read `style`" (its + * out-of-window fallback would start honouring it) and the + * non-recursion case (`{ style: 'relative' }` blows the stack). + */ +import { describe, it, expect } from 'vitest'; + +import { formatDate, formatDateTime, formatRelativeDate } from '../date-display'; + +/** The card's instant and locale, in the card's face. A non-current year, so + * the "drop the year" branch of the default face is not in play. */ +const V = '2024-07-04T07:00:00.000Z'; +const L = 'en-US'; + +/** A date INSIDE the ±7-day relative window, so the relative path produces a + * relative phrase rather than delegating to the absolute face. */ +function inTwoDays(): Date { + const d = new Date(); + d.setDate(d.getDate() + 2); + return d; +} + +describe('formatDate reads options.style (#7745)', () => { + it('the options spelling is read at all — the card\'s before/after reading', () => { + // Before this change this was `Jul 4, 2024`: the default face, no diagnostic. + expect(formatDate(V, undefined, { style: 'short', locale: L })).toBe("Jul 4, '24"); + }); + + it('the options spelling IS the positional spelling, for every face', () => { + // Stated as an equivalence rather than three literals: the property is + // "one concept, one behaviour", so a redesign of a face moves both sides. + for (const style of ['short', 'relative', 'long', 'compact'] as const) { + expect(formatDate(V, undefined, { style, locale: L })).toBe(formatDate(V, style, { locale: L })); + } + // ... anchored by the two literals the card measured, so a redesign that + // moved both sides together could not pass silently. + expect(formatDate(V, undefined, { style: 'short', locale: L })).toBe("Jul 4, '24"); + expect(formatDate(V, undefined, { style: 'long', locale: L })).toBe('Jul 4, 2024'); + }); + + it('reaches the relative branch too, not just the short one', () => { + const soon = inTwoDays(); + expect(formatDate(soon, undefined, { style: 'relative', locale: L })).toBe( + formatRelativeDate(soon, { locale: L }), + ); + // And that is genuinely the relative phrase, not the absolute face. + expect(formatDate(soon, undefined, { style: 'relative', locale: L })).not.toBe( + formatDate(soon, undefined, { locale: L }), + ); + }); +}); + +describe('the precedence between the two spellings is pinned: positional wins (#7745)', () => { + it('the positional argument beats options.style', () => { + expect(formatDate(V, 'short', { style: 'long', locale: L })).toBe("Jul 4, '24"); + expect(formatDate(V, 'long', { style: 'short', locale: L })).toBe('Jul 4, 2024'); + }); + + it('beats it on the sharpest pair — where the loser would be VISIBLY different', () => { + const soon = inTwoDays(); + // options-wins would render the relative phrase ("In 2 days") here. + expect(formatDate(soon, 'short', { style: 'relative', locale: L })).toBe( + formatDate(soon, 'short', { locale: L }), + ); + // options-wins would render the compact-year short face here. + expect(formatDate(soon, 'relative', { style: 'short', locale: L })).toBe( + formatRelativeDate(soon, { locale: L }), + ); + }); + + it('"absent" means `undefined`, not "falsy" — `\'\'` still counts as given', () => { + // `??`, not `||`. `formatDate(v, '', bag)` renders the default face today + // and must keep doing so; falling through to the key would change it. + expect(formatDate(V, '', { style: 'short', locale: L })).toBe('Jul 4, 2024'); + expect(formatDate(V, '', { style: 'short', locale: L })).toBe(formatDate(V, '', { locale: L })); + }); + + it('the positional slot still exists — arity pin', () => { + // Turns RED the moment the positional parameter is dropped, which would + // silently move every `formatDate(v, 'short', opts)` caller's arguments. + expect(formatDate.length).toBe(3); + }); +}); + +describe('nothing that renders a face today moves (#7745 is additive)', () => { + it('every positional call renders exactly what it rendered before', () => { + expect(formatDate(V, 'short', { locale: L })).toBe("Jul 4, '24"); + expect(formatDate(V, undefined, { locale: L })).toBe('Jul 4, 2024'); + expect(formatDate(V, 'relative', { locale: L })).toBe('Jul 4, 2024'); + expect(formatDate('', undefined, { locale: L })).toBe('—'); + expect(formatDate('not a date', undefined, { locale: L })).toBe('—'); + }); + + it('the two OTHER inert keys stay exactly as inert as they were', () => { + // `dueLike` is read by `formatRelativeDate` alone, `t` likewise. #7745 + // touches neither, and the reviewer's regression reading is the pin. + expect(formatDateTime(V, { dueLike: true, t: () => 'X', locale: L })).toBe('Jul 4, 2024, 07:00 AM'); + expect(formatDate(V, undefined, { dueLike: true, t: () => 'X', locale: L })).toBe('Jul 4, 2024'); + }); + + it('formatDateTime keeps its own reading of the same key', () => { + expect(formatDateTime(V, { style: 'compact', locale: L })).toBe('7/4/2024 7:00 am'); + expect(formatDateTime(V, { locale: L })).toBe('Jul 4, 2024, 07:00 AM'); + }); + + it('a bag built for formatDateTime lands on formatDate as the default face', () => { + // The shared bag can legitimately carry a sibling's key. `'compact'` is + // not `formatDate`'s vocabulary, so it is "anything else" — the default. + expect(formatDate(V, undefined, { style: 'compact', locale: L })).toBe('Jul 4, 2024'); + }); +}); + +describe('formatRelativeDate still does not read `style` (#7745 does not touch it)', () => { + it('ignores it inside the ±7-day window', () => { + const soon = inTwoDays(); + expect(formatRelativeDate(soon, { style: 'short', locale: L })).toBe( + formatRelativeDate(soon, { locale: L }), + ); + }); + + it('ignores it on the out-of-window ABSOLUTE fallback too', () => { + // This is the edge `formatDate`'s new read would have leaked through: + // beyond ±7 days `formatRelativeDate` delegates to `formatDate`, so + // without the strip the key would take effect HERE and nowhere else. + expect(formatRelativeDate(V, { style: 'short', locale: L })).toBe('Jul 4, 2024'); + expect(formatRelativeDate(V, { style: 'compact', locale: L })).toBe('Jul 4, 2024'); + }); + + it('does not recurse when the bag carries `style: \'relative\'`', () => { + // `formatDate` resolves `'relative'` by calling `formatRelativeDate`, + // which delegates back for out-of-window dates. With the style still in + // the bag that is an unbounded loop, not a face. + expect(() => formatRelativeDate(V, { style: 'relative', locale: L })).not.toThrow(); + expect(formatRelativeDate(V, { style: 'relative', locale: L })).toBe('Jul 4, 2024'); + expect(() => formatDate(V, undefined, { style: 'relative', locale: L })).not.toThrow(); + expect(formatDate(V, undefined, { style: 'relative', locale: L })).toBe('Jul 4, 2024'); + }); +}); diff --git a/packages/core/src/utils/date-display.ts b/packages/core/src/utils/date-display.ts index 8d5716224c..6cbabb21cc 100644 --- a/packages/core/src/utils/date-display.ts +++ b/packages/core/src/utils/date-display.ts @@ -50,7 +50,7 @@ * Options shared by {@link formatDate} / {@link formatRelativeDate} / * {@link formatDateTime}. One bag, and each function reads the keys it needs: * `dueLike` and `t` only matter on the relative path, `style` is read by - * `formatDateTime` only (see below). + * `formatDateTime` and by `formatDate` (see below). */ export interface DateDisplayOptions { dueLike?: boolean; @@ -59,18 +59,28 @@ export interface DateDisplayOptions { /** i18n translate fn for phrases `Intl` can't produce (the "Overdue Nd" wording). */ t?: (key: string, params?: Record) => string; /** - * Named face, read by {@link formatDateTime}: `'compact'` is the dense grid - * cell face (objectui#7443); anything else, or absent, is the default face. + * Named face, read by {@link formatDateTime} and {@link formatDate}. Each + * reads its own vocabulary: `'compact'` is `formatDateTime`'s dense grid + * cell face (objectui#7443), `'short'` and `'relative'` are `formatDate`'s; + * anything else, or absent, is that function's default face. * * It rides here rather than in a second positional parameter because * `formatDateTime(value, options?)` is a PUBLISHED signature with `options` * in position two (objectui#4272). A positional `style` would have displaced * it: TypeScript would reject the old call, but a JavaScript caller would * silently hand its options bag to the style slot and lose its locale — - * the #4272 defect again. {@link formatDate} still takes its style - * positionally and does NOT read this key; the symmetric long-run shape - * (both functions reading `options.style`) is additive on `formatDate` and - * deliberately not part of #7443. + * the #4272 defect again. + * + * ⚠️ On {@link formatDate} this key COLLIDES with a positional parameter of + * the same name, so the precedence is PINNED, not left to implementation + * order: **the positional argument wins**, and this key is consulted only + * when the positional slot is `undefined` (objectui#7745). See + * {@link formatDate}'s own note for why that direction and not the other. + * + * ⚠️ {@link formatRelativeDate} still does NOT read this key — the + * maintainer's long-run ruling on objectui#7443 names `formatDate` only, and + * whether the relative path's out-of-window fallback should honour it is a + * separate, deliberate call (objectui#7745's report). */ style?: string; } @@ -92,6 +102,31 @@ function formatRelativeDays(diffDays: number, locale?: string): string { } } +/** + * The options bag {@link formatRelativeDate} hands to {@link formatDate} for + * its out-of-window ABSOLUTE fallback, with `style` neutralised. + * + * Two reasons, both load-bearing since objectui#7745 made `formatDate` read + * `options.style`: + * + * 1. **Behaviour preservation.** `formatRelativeDate` does not read + * `options.style`, and #7745 does not change that. Without the strip it + * would start reading it THROUGH this delegation for dates outside the + * ±7-day window only — a face change on a live path (grid cell, gantt + * tooltip) that no card authorises. + * 2. **Termination.** `formatDate` resolves `'relative'` by calling + * `formatRelativeDate`, which lands back here. With the style still in + * the bag, `formatRelativeDate(v, { style: 'relative' })` on an + * out-of-window date would recurse until the stack ran out. + * + * The bag is returned UNCHANGED when there is nothing to strip, so the common + * path allocates nothing. + */ +function absoluteFallbackOptions(options?: DateDisplayOptions): DateDisplayOptions | undefined { + if (options === undefined || options.style === undefined) return options; + return { ...options, style: undefined }; +} + /** * Format date as relative time (e.g., "3 days ago", "Today", "Overdue 3d"), * localized via `Intl.RelativeTimeFormat` (objectstack-ai/objectstack#3040). @@ -114,7 +149,7 @@ export function formatRelativeDate(value: string | Date | number, options?: Date const diffDays = Math.round(diffMs / (1000 * 60 * 60 * 24)); // Beyond the ±7-day window, fall back to the absolute (already localized) form. - if (diffDays < -7 || diffDays > 7) return formatDate(date, undefined, options); + if (diffDays < -7 || diffDays > 7) return formatDate(date, undefined, absoluteFallbackOptions(options)); if (diffDays < -1 && options?.dueLike) { const absDays = Math.abs(diffDays); @@ -126,14 +161,48 @@ export function formatRelativeDate(value: string | Date | number, options?: Date } /** - * Format date value + * Format date value. + * + * The named face comes from EITHER spelling — the positional `style` + * parameter or `options.style` — and the precedence between them is pinned + * (objectui#7745): + * + * > **The positional argument wins. `options.style` is consulted only when the + * > positional slot is `undefined`.** + * + * Before #7745 only the positional spelling was read here, while + * {@link formatDateTime} read only the `options` one — one shared bag, two + * spellings for one concept, and on THIS function the options spelling did + * nothing at all: `formatDate(v, undefined, { style: 'short' })` silently + * rendered the default face with no diagnostic. Reading the key is the + * additive half of the maintainer's long-run ruling on objectui#7443. + * + * Why the positional wins, and not the newer key: + * + * - It is the ONLY direction that is purely additive. It fires exactly on + * the input that is a silent no-op today (positional absent, key present); + * every call that renders a face today renders the same face after. + * - The bag is SHARED across three functions, so it can legitimately carry a + * key meant for a sibling — that is this module's convention (`dueLike` + * and `t` are read by `formatRelativeDate` alone). A caller that built + * `{ style: 'compact', locale }` for `formatDateTime` and reused the bag + * for `formatDate(v, 'short', bag)` must keep its short face. A key aimed + * at a sibling function must not outrank an argument written for THIS + * call — that is objectui#7694's shape (an alias overwriting the canonical + * key), and the silent-override half of objectui#4272. + * + * `??`, not `||`, is what "absent" means here: `formatDate(v, '', bag)` + * renders the default face today and must keep doing so, so an empty string + * counts as GIVEN and does not fall through to the key. */ export function formatDate(value: string | Date | number, style?: string, options?: DateDisplayOptions): string { if (value === null || value === undefined || value === '') return '—'; const date = value instanceof Date ? value : new Date(value as any); if (!(date instanceof Date) || isNaN(date.getTime())) return '—'; - if (style === 'short') { + const effectiveStyle = style ?? options?.style; + + if (effectiveStyle === 'short') { // Compact format for mobile: "Jan 15, '24" / "1月 15, '24". // Only the MONTH token is localized: the surrounding compact shape (day, // apostrophe + 2-digit year) is a deliberate fixed layout for narrow @@ -146,7 +215,7 @@ export function formatDate(value: string | Date | number, style?: string, option return `${month} ${day}, '${year}`; } - if (style === 'relative') { + if (effectiveStyle === 'relative') { return formatRelativeDate(date, options); }