From 035464d16bca7f3cf4cef913021c937e0296173d Mon Sep 17 00:00:00 2001 From: Marie Chatfield Date: Fri, 8 May 2026 12:53:24 -0600 Subject: [PATCH 1/3] docs(rfc): add SDK-810 date normalization audit and solution spike Consolidates the timezone bug audit and two-phase solution proposal into rfc/2026-05-11--date-normalization.md. Covers known unsafe callsites, the Phase 1 convention-hardening plan, and the Phase 2 CalendarDate migration approach. Co-Authored-By: Claude Sonnet 4.6 --- rfc/2026-05-11--date-normalization.md | 226 ++++++++++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 rfc/2026-05-11--date-normalization.md diff --git a/rfc/2026-05-11--date-normalization.md b/rfc/2026-05-11--date-normalization.md new file mode 100644 index 000000000..d5aea1584 --- /dev/null +++ b/rfc/2026-05-11--date-normalization.md @@ -0,0 +1,226 @@ +# SDK-810: Date Normalization + +Every UTC bug in this codebase has the same root cause: `Date` carries a time and timezone component, but we use it to represent calendar dates that have neither. All the workarounds — `normalizeDateToLocal`, the regex-parse path in `normalizeToDate`, the `getFullYear/getMonth/getDate` read pattern — exist because of this mismatch. + +The fix has two phases. Phase 1 eliminates all known bugs by hardening the existing convention. Phase 2 migrates the internal type to `CalendarDate`, which makes correct behavior structural rather than conventional. Phase 1 is a prerequisite for Phase 2 — the callsite fixes done in Phase 1 are required work either way. + +--- + +## Audit + +### Libraries + +| Library | Purpose | +| ------------------------- | ---------------------------------------------------------- | +| `@internationalized/date` | `CalendarDate` / `DateValue` — react-aria picker internals | +| `@gusto/embedded-api` | `RFCDate` — wraps dates for API submission | +| _(none)_ | No date-fns, dayjs, moment, or luxon | + +All date manipulation outside the picker layer uses native JS `Date` and `Intl` APIs. + +### Date Types in Use + +#### 1. JavaScript `Date` Object + +Primary internal representation. Used for state, props, business logic, and class properties throughout the codebase. + +Key serialization operations and their timezone safety: + +| Operation | Method | Timezone-safe? | +| ---------------------- | --------------------------------------------- | -------------- | +| Extract year/month/day | `.getFullYear()`, `.getMonth()`, `.getDate()` | ✅ Local | +| Convert to YYYY-MM-DD | `.toISOString().split('T')[0]` | ⚠️ Reads UTC | +| Locale-aware display | `.toLocaleDateString(locale, options)` | ✅ Local | +| Set time to midnight | `.setHours(0, 0, 0, 0)` | ✅ Local | + +#### 2. `CalendarDate` / `DateValue` (`@internationalized/date`) + +Used **only inside picker components** as the format required by react-aria. `CalendarDate` is `{ year, month, day }` with no timezone. + +Both `DatePicker.tsx` and `DateRangePicker.tsx` define identical local bridge functions (duplicated): + +``` +Date → getFullYear/getMonth/getDate → "YYYY-MM-DD" → parseDate() → CalendarDate +CalendarDate → { year, month, day } → new Date(year, month-1, day) → Date +``` + +`CalendarPreview.tsx` has two unsafe patterns: `formatDateToStringDate(highlight.date)` and `formatDateToStringDate(dateRange.start/end)` use `.toISOString()` ⚠️; the `isInRange` helper does `new Date(date.toString())` where `date` is a `DateValue` — `CalendarDate.toString()` yields `"YYYY-MM-DD"`, which `new Date()` parses as UTC midnight, making the range comparison wrong for UTC+ users ⚠️. + +#### 3. Date Strings (`YYYY-MM-DD`) + +The API boundary format. Functions that produce them: + +| Function | Method | Safe? | +| ---------------------------------------------------------- | ------------------------------ | ----- | +| `normalizeToISOString` | `getFullYear/getMonth/getDate` | ✅ | +| `formatDateToStringDate` | `.toISOString().split('T')[0]` | ⚠️ | +| `coerceToISODate`, `toISODateString`, several inline sites | `.toISOString().split('T')[0]` | ⚠️ | + +Functions that parse them: `normalizeToDate` handles `YYYY-MM-DD` safely via regex + local-midnight constructor. Inline `new Date(apiString)` elsewhere reads UTC midnight ⚠️. + +#### 4. `RFCDate` (`@gusto/embedded-api`) + +Wraps dates for API write mutations. Serializes internally via `.toISOString().slice(0, 10)` (UTC). + +- **Safe:** `new RFCDate("YYYY-MM-DD")` — string input is UTC midnight; serializes back correctly +- **Unsafe:** `new RFCDate(new Date(someString))` — local-midnight `Date` is UTC midnight minus the offset; wrong day for UTC+ users ⚠️ + +Three files use the unsafe pattern: `useEmployeeDetailsForm` (`dateOfBirth`), `useHomeAddressForm` (`effectiveDate`), `useWorkAddressForm` (`effectiveDate`). + +### Central Utilities + +**`src/helpers/dateFormatting.ts`** — all date utilities. Notable: + +- `normalizeToDate` — parses `YYYY-MM-DD` to local-midnight `Date` ✅; falls through to `new Date(string)` for other formats (e.g. ISO timestamps) ⚠️ — callers that pass non-date strings inherit UTC parsing +- `normalizeToISOString` — safe serialization via local getters ✅; only accepts `string`, not `Date` +- `formatDateToStringDate` — unsafe serialization via `.toISOString()` ⚠️ +- `normalizeDateToLocal` — patches UTC-midnight `Date` to local; exists solely to work around `formatDateToStringDate` + +**`src/hooks/useDateFormatter.ts`** — React hook wrapping all display formatters with the current locale. + +### Known Timezone Issues + +| Location | Issue | +| -------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | +| `formatDateToStringDate` (`dateFormatting.ts:171`) | `.toISOString()` reads UTC | +| `coerceToISODate` (`preprocessors.ts:10`) | `.toISOString()` reads UTC | +| `toISODateString` (`useDateRangeFilter.ts:26`) | `.toISOString()` reads UTC | +| `PaymentsList.tsx:34-35` | Inline `.toISOString()` reads UTC | +| `useEmployeeStateTaxesForm.tsx:289` | `.toISOString()` reads UTC | +| `PayPeriod.fromPayPeriodData` | `new Date(YYYY-MM-DD)` → UTC midnight | +| `Payroll/helpers.ts` (compensation effective dates) | `new Date(effectiveDate)` on string → UTC midnight | +| `useEmployeeStateTaxesForm` deserialization | `new Date(trimmed)` on string → UTC midnight | +| `CalendarPreview.tsx:32` | `new Date(dateValue.toString())` — behavior varies | +| `useEmployeeDetailsForm`, `useHomeAddressForm`, `useWorkAddressForm` | `new RFCDate(new Date(string))` — UTC roundtrip (**live data-integrity bug** for UTC+ users: wrong birth date / effective date sent to API) | +| `getHoursUntil`, `getDaysUntil` (`dateFormatting.ts`) | Pass string to `normalizeToDate`, which falls through to `new Date(string)` for ISO timestamps → UTC parsing; arithmetic is off for UTC+ users | + +### Notable Domain Patterns + +**Payroll (`canCancelPayroll`):** Hand-rolled DST-aware Pacific Time offset for a 4 PM PT deadline check. The implementation offsets both `now` and `deadline` into a "fake PT" coordinate system (subtracting 7 or 8 hours from their UTC timestamps), then calls `.setUTCHours(16, 0, 0, 0)` on the pre-shifted deadline to pin the cutoff to fake-4-PM. Because both sides of the comparison are in the same shifted system, the comparison is self-consistent and produces the correct 4 PM PT cutoff in the common case. However, the logic is fragile in two ways: (1) the DST boundary check uses midnight local time rather than 2 AM, so the PT offset can be wrong by 1 hour in a narrow window around the transition; (2) if `now` and `payrollDeadline` straddle a DST transition they receive different offsets (-7 vs -8), introducing a 1-hour error. These edge cases need explicit investigation and test coverage before any migration touches this function. + +**`PayPeriod` model:** Stores `Date` via `new Date(data.start_date)` (⚠️ UTC midnight). Display via `toLocaleDateString` reads local time, so UTC+ users may see the previous day. + +**Off-cycle min check date:** `new Date()` + `setHours(0,0,0,0)` + `addBusinessDays` — local midnight consistently ✅. + +**Address pending-change detection:** `normalizeToDate` + `startOfLocalDay` comparison — timezone-safe ✅. + +**Holiday math:** Pure `new Date(year, month, day)` arithmetic with no string parsing — correct ✅. + +### Summary + +| Date type | Where used | Notes | +| ------------------- | ----------------------------------------- | --------------------------- | +| `Date` | Everywhere — state, props, business logic | Primary internal type | +| `CalendarDate` | Picker UI layer only | react-aria internal | +| `DateValue` | Picker UI layer only | react-aria callback type | +| `RFCDate` | API write boundary | Safe only with string input | +| `YYYY-MM-DD` string | API boundary, form state | To/from API | +| ISO timestamp | `payrollDeadline` and some API fields | Includes time component | + +--- + +## Phase 1: Harden the Existing `Date` Convention + +### What Changes + +**Fix `new RFCDate(new Date(string))` patterns** — replace with `new RFCDate(string)` in 3 files: `useEmployeeDetailsForm` (`dateOfBirth`), `useHomeAddressForm` (`effectiveDate`), `useWorkAddressForm` (`effectiveDate`). These are live data-integrity bugs: UTC+ users have the wrong date written to the Gusto API. Do this first — it is the lowest-risk change and the highest-severity bug. + +**Extend `normalizeToISOString`** to accept `Date | string | null` (currently only `string | null`). Implementation reads `getFullYear/getMonth/getDate` for both input types — no UTC. + +**Delete `formatDateToStringDate`** and **`normalizeDateToLocal`** (only ever called to work around `formatDateToStringDate`). + +**Replace ~12 broken serialization callsites:** + +| File | Was | Becomes | +| ----------------------------------------------------------------------- | --------------------------------------------------------- | ------------------------------------------------ | +| `DatePickerField.tsx` | `normalizeDateToLocal(v)` → `formatDateToStringDate(...)` | `normalizeToISOString(v)` | +| `DatePicker.tsx` | `formatDateToStringDate(value)` → `parseDate(...)` | `normalizeToISOString(value)` → `parseDate(...)` | +| `CalendarPreview.tsx` | `formatDateToStringDate(date)` | `normalizeToISOString(date)` | +| `preprocessors.ts` | `val.toISOString().split('T')[0]` | `normalizeToISOString(val)` | +| `useDateRangeFilter.ts` | `date.toISOString().split('T')[0]` | `normalizeToISOString(date)` | +| `PaymentsList.tsx` | `new Date().toISOString().split('T')[0]` | `normalizeToISOString(new Date())` | +| `useEmployeeStateTaxesForm.tsx` | `formValue.toISOString().split('T')[0]` | `normalizeToISOString(formValue)` | +| _(+ 4 more: signatory, pay schedule, contractor profile, payroll list)_ | — | — | + +**Fix `new Date(YYYY-MM-DD)` deserialization patterns** — replace with `normalizeToDate(apiString)` in `PayPeriod.fromPayPeriodData`, compensation sorting, and state taxes deserialization. + +**Fix `normalizeToDate` fallthrough** — the function parses `YYYY-MM-DD` safely but falls through to `new Date(string)` for all other formats. Callers that pass ISO timestamps (`getHoursUntil`, `getDaysUntil`) inherit UTC parsing. Either restrict the function to `YYYY-MM-DD` only and reject other strings, or add an explicit safe path for timestamps via a separate `normalizeToDateTime` utility. + +### What Stays the Same + +All type signatures, partner hook APIs, `DatePickerProps`, arithmetic helpers, `PayPeriod` model shape. No breaking changes. + +### Implementation Order + +1. `new RFCDate(new Date(string))` → `new RFCDate(string)` in 3 employee form files +2. Extend `normalizeToISOString`; delete `formatDateToStringDate` and `normalizeDateToLocal`; replace serialization callsites +3. Fix `new Date(YYYY-MM-DD)` deserialization callsites +4. Address `normalizeToDate` fallthrough for timestamp strings + +--- + +## Phase 2: `CalendarDate` as the Internal Date Type + +Phase 1 fixes current bugs by convention. Phase 2 makes the correct behavior impossible to violate: `CalendarDate` has no time component and no UTC methods, so the class of bug cannot arise. + +### Adapter Boundary Is Fixed + +`DatePickerProps` and `DateRangePickerProps` are the partner-facing API and must keep `Date` at the boundary. Everything above this boundary is internal and can change. + +Conversions happen at two edges: + +- **Inbound** (adapter → form): `value.getFullYear/getMonth/getDate` → `new CalendarDate(...)` — reads local, safe +- **Outbound** (form → adapter): `new Date(cd.year, cd.month - 1, cd.day)` — local midnight, safe +- **Display**: `cd.toDate(getLocalTimeZone()).toLocaleDateString(...)` + +### What Changes + +**`PayPeriod` model:** `parseDate(data.start_date)` → `CalendarDate` instead of `new Date(data.start_date)` (UTC midnight). + +**Compensation sorting:** `parseDate(a.effectiveDate).compare(parseDate(b.effectiveDate))` instead of `new Date(effectiveDate).getTime()`. + +**Address pending-change detection:** `parseDate(raw).compare(today(getLocalTimeZone())) > 0` replaces `normalizeToDate` + `startOfLocalDay`. + +**`useDateRangeFilter`:** State becomes `CalendarDate | null`; arithmetic via `.add({ months: n })`; serialization via `.toString()`. + +**State taxes date fields:** Deserialization via `parseDate(trimmed)`, serialization via `formValue.toString()`. + +**`canCancelPayroll`** — the current implementation must be investigated before this migration. The hand-rolled DST logic shifts both `now` and `deadline` into a "fake PT" coordinate system (subtracting 7 or 8 hours), then uses `.setUTCHours(16, 0, 0, 0)` to pin the cutoff to 4 PM in that shifted system. This is self-consistent in the common case but has two edge-case failure modes: the DST boundary is computed at midnight local (not 2 AM), and if `now` and `payrollDeadline` straddle a DST transition they get different offsets, producing a 1-hour error. Once the intended semantics and failure modes are confirmed against tests and the API contract, the correct replacement is: + +```ts +const nowPT = now('America/Los_Angeles') +const deadlinePT = parseAbsolute(payroll.payrollDeadline.toISOString(), 'America/Los_Angeles') +const cutoffPT = deadlinePT.set({ hour: 16, minute: 0, second: 0, millisecond: 0 }) +return nowPT.compare(cutoffPT) < 0 +``` + +Note: `deadlinePT.set({ hour: 16 })` sets 4 PM on the deadline's _date in PT_. This is only correct if the intent is "4 PM on the same calendar date as `payrollDeadline` in PT" — confirm this against the API contract before adopting this replacement. + +### What Gets Removed + +- `normalizeDateToLocal` — already removed in Phase 1 +- `formatDateToStringDate` — already removed in Phase 1 +- `normalizeToISOString` — replaced by `parseDate(s).toString()` +- `getPacificTimeOffset` — replaced by `ZonedDateTime` +- `addDays`, `isWeekend` — replaced by `.add({ days: n })` and `dayOfWeek >= 6` +- `startOfLocalDay` (duplicated in two address files) +- Duplicated `dateToCalendarDate` / `calendarDateValueToDate` in both picker files + +### Risks + +**1. Form value type change could be breaking.** If `DatePickerField`'s generic changes from `Date | null` to `CalendarDate | null`, partners reading form values directly would see a type change. _Mitigation: use `CalendarDate` as a safe intermediate, keep `Date` stored in form state — fixes all bugs with zero breaking changes._ + +**2. `dayOfWeek` indexing differs.** `CalendarDate.dayOfWeek`: Mon=1, Sun=7. `Date.getDay()`: Sun=0, Sat=6. A direct port of `isWeekend` gets this wrong. _Mitigation: a single `isCalendarWeekend` utility._ + +**3. Large migration surface.** Touches nearly every date-handling file. Requires test coverage before migrating each area. + +### Open Question: Where Does `CalendarDate` Live in Form State? + +- **Conservative:** use `CalendarDate` as a safe intermediate only; keep `Date` in form state. Fixes all bugs, zero breaking changes, `isStringMode` survives unchanged. +- **Progressive:** store `CalendarDate` in form state. Eliminates `isStringMode`, cleaner long-term, but is technically a breaking change. + +`isStringMode` exists because `DatePickerField` is generic over `Date | null` _or_ `string | null`. Partners that bind date fields to a Zod schema where values round-trip as strings rely on the string output path. Before taking the progressive path, confirm no partner uses `DatePickerField` — check SDK documentation and partner-facing examples. + +### Implementation Order + +`DatePickerField` → `PayPeriod` → address utilities → `canCancelPayroll` (after bug investigation) → remaining form files From 4b7be152aec41046f663ffb8cfff7ff849e7da36 Mon Sep 17 00:00:00 2001 From: Marie Chatfield Date: Mon, 11 May 2026 14:08:24 -0700 Subject: [PATCH 2/3] docs(rfc): update date normalization spike with SDK-828 learnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Correct the RFCDate section: new RFCDate(new Date(string)) is a harmless UTC roundtrip when the input is a YYYY-MM-DD string, not a live bug. Fields in the three affected hooks are typed as z.iso.date() so the UTC→UTC cancellation holds. - Demote the three-hook cleanup from highest-priority bug fix to optional cleanup in the Phase 1 implementation order. - Note that SDK-828 (PR #1767) fixes formatDateToStringDate (Phase 1 step 2 partial) but leaves callsite migration and deserialization fixes (steps 3–4) as the remaining live bugs. Co-Authored-By: Claude Sonnet 4.6 --- rfc/2026-05-11--date-normalization.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/rfc/2026-05-11--date-normalization.md b/rfc/2026-05-11--date-normalization.md index d5aea1584..49daddf52 100644 --- a/rfc/2026-05-11--date-normalization.md +++ b/rfc/2026-05-11--date-normalization.md @@ -63,9 +63,10 @@ Functions that parse them: `normalizeToDate` handles `YYYY-MM-DD` safely via reg Wraps dates for API write mutations. Serializes internally via `.toISOString().slice(0, 10)` (UTC). - **Safe:** `new RFCDate("YYYY-MM-DD")` — string input is UTC midnight; serializes back correctly -- **Unsafe:** `new RFCDate(new Date(someString))` — local-midnight `Date` is UTC midnight minus the offset; wrong day for UTC+ users ⚠️ +- **Safe (but redundant):** `new RFCDate(new Date("YYYY-MM-DD"))` — `new Date` parses the string as UTC midnight; `RFCDate` serializes back via UTC; roundtrip is correct. The intermediate `Date` is unnecessary and confusing, but not buggy. +- **Unsafe:** `new RFCDate(new Date(localMidnightDate))` — if the `Date` was constructed via `new Date(year, month, day)` (local midnight), then UTC serialization reads the previous day for UTC+ users ⚠️ -Three files use the unsafe pattern: `useEmployeeDetailsForm` (`dateOfBirth`), `useHomeAddressForm` (`effectiveDate`), `useWorkAddressForm` (`effectiveDate`). +Three files use `new RFCDate(new Date(string))`: `useEmployeeDetailsForm` (`dateOfBirth`), `useHomeAddressForm` (`effectiveDate`), `useWorkAddressForm` (`effectiveDate`). Because these fields are typed as `z.iso.date()` in their Zod schemas, the value is always a `YYYY-MM-DD` string — so the UTC roundtrip cancels and no data-integrity bug occurs. The pattern is redundant noise, not a bug. ### Central Utilities @@ -91,7 +92,7 @@ Three files use the unsafe pattern: `useEmployeeDetailsForm` (`dateOfBirth`), `u | `Payroll/helpers.ts` (compensation effective dates) | `new Date(effectiveDate)` on string → UTC midnight | | `useEmployeeStateTaxesForm` deserialization | `new Date(trimmed)` on string → UTC midnight | | `CalendarPreview.tsx:32` | `new Date(dateValue.toString())` — behavior varies | -| `useEmployeeDetailsForm`, `useHomeAddressForm`, `useWorkAddressForm` | `new RFCDate(new Date(string))` — UTC roundtrip (**live data-integrity bug** for UTC+ users: wrong birth date / effective date sent to API) | +| `useEmployeeDetailsForm`, `useHomeAddressForm`, `useWorkAddressForm` | `new RFCDate(new Date(string))` — redundant UTC roundtrip, but **not a bug**: fields are `z.iso.date()` strings so UTC→UTC conversion cancels correctly | | `getHoursUntil`, `getDaysUntil` (`dateFormatting.ts`) | Pass string to `normalizeToDate`, which falls through to `new Date(string)` for ISO timestamps → UTC parsing; arithmetic is off for UTC+ users | ### Notable Domain Patterns @@ -123,7 +124,7 @@ Three files use the unsafe pattern: `useEmployeeDetailsForm` (`dateOfBirth`), `u ### What Changes -**Fix `new RFCDate(new Date(string))` patterns** — replace with `new RFCDate(string)` in 3 files: `useEmployeeDetailsForm` (`dateOfBirth`), `useHomeAddressForm` (`effectiveDate`), `useWorkAddressForm` (`effectiveDate`). These are live data-integrity bugs: UTC+ users have the wrong date written to the Gusto API. Do this first — it is the lowest-risk change and the highest-severity bug. +**Clean up `new RFCDate(new Date(string))` patterns** — replace with `new RFCDate(string)` in 3 files: `useEmployeeDetailsForm` (`dateOfBirth`), `useHomeAddressForm` (`effectiveDate`), `useWorkAddressForm` (`effectiveDate`). These fields are `z.iso.date()` strings, so the intermediate `Date` is a harmless UTC roundtrip rather than a live bug. The change is optional cleanup for clarity — not a correctness fix. **Extend `normalizeToISOString`** to accept `Date | string | null` (currently only `string | null`). Implementation reads `getFullYear/getMonth/getDate` for both input types — no UTC. @@ -152,10 +153,10 @@ All type signatures, partner hook APIs, `DatePickerProps`, arithmetic helpers, ` ### Implementation Order -1. `new RFCDate(new Date(string))` → `new RFCDate(string)` in 3 employee form files -2. Extend `normalizeToISOString`; delete `formatDateToStringDate` and `normalizeDateToLocal`; replace serialization callsites -3. Fix `new Date(YYYY-MM-DD)` deserialization callsites -4. Address `normalizeToDate` fallthrough for timestamp strings +1. Extend `normalizeToISOString`; delete `formatDateToStringDate` and `normalizeDateToLocal`; replace serialization callsites +2. Fix `new Date(YYYY-MM-DD)` deserialization callsites +3. Address `normalizeToDate` fallthrough for timestamp strings +4. _(Optional cleanup)_ `new RFCDate(new Date(string))` → `new RFCDate(string)` in 3 employee form files — not a bug, but removes confusing indirection --- From d984787a39eae27b74f674d7964c91ef7114e621 Mon Sep 17 00:00:00 2001 From: Marie Chatfield Date: Tue, 12 May 2026 18:54:29 -0700 Subject: [PATCH 3/3] docs: address feedback for string API and less reliance on library --- rfc/2026-05-11--date-normalization.md | 166 +++++++++++++++++++++----- 1 file changed, 134 insertions(+), 32 deletions(-) diff --git a/rfc/2026-05-11--date-normalization.md b/rfc/2026-05-11--date-normalization.md index 49daddf52..2180ce056 100644 --- a/rfc/2026-05-11--date-normalization.md +++ b/rfc/2026-05-11--date-normalization.md @@ -2,7 +2,7 @@ Every UTC bug in this codebase has the same root cause: `Date` carries a time and timezone component, but we use it to represent calendar dates that have neither. All the workarounds — `normalizeDateToLocal`, the regex-parse path in `normalizeToDate`, the `getFullYear/getMonth/getDate` read pattern — exist because of this mismatch. -The fix has two phases. Phase 1 eliminates all known bugs by hardening the existing convention. Phase 2 migrates the internal type to `CalendarDate`, which makes correct behavior structural rather than conventional. Phase 1 is a prerequisite for Phase 2 — the callsite fixes done in Phase 1 are required work either way. +The fix has three phases. Phase 1 eliminates all known bugs by hardening the existing convention. Phase 2 introduces an internal `ISODate` class that wraps a `YYYY-MM-DD` string, replacing unsafe `Date` construction in models, form state, and utilities — no partner-facing changes. Phase 3 extends the string contract to the partner-facing API as a breaking change, exposing lightweight `ISOString` utilities for cases where partners need to construct date strings. Phase 1 is a prerequisite for the rest — the callsite fixes done in Phase 1 are required work either way. --- @@ -91,7 +91,7 @@ Three files use `new RFCDate(new Date(string))`: `useEmployeeDetailsForm` (`date | `PayPeriod.fromPayPeriodData` | `new Date(YYYY-MM-DD)` → UTC midnight | | `Payroll/helpers.ts` (compensation effective dates) | `new Date(effectiveDate)` on string → UTC midnight | | `useEmployeeStateTaxesForm` deserialization | `new Date(trimmed)` on string → UTC midnight | -| `CalendarPreview.tsx:32` | `new Date(dateValue.toString())` — behavior varies | +| `CalendarPreview.tsx:32` | `new Date(dateValue.toString())` — `CalendarDate.toString()` yields `YYYY-MM-DD`, parsed as UTC midnight; off by one for UTC+ users | | `useEmployeeDetailsForm`, `useHomeAddressForm`, `useWorkAddressForm` | `new RFCDate(new Date(string))` — redundant UTC roundtrip, but **not a bug**: fields are `z.iso.date()` strings so UTC→UTC conversion cancels correctly | | `getHoursUntil`, `getDaysUntil` (`dateFormatting.ts`) | Pass string to `normalizeToDate`, which falls through to `new Date(string)` for ISO timestamps → UTC parsing; arithmetic is off for UTC+ users | @@ -153,38 +153,73 @@ All type signatures, partner hook APIs, `DatePickerProps`, arithmetic helpers, ` ### Implementation Order -1. Extend `normalizeToISOString`; delete `formatDateToStringDate` and `normalizeDateToLocal`; replace serialization callsites -2. Fix `new Date(YYYY-MM-DD)` deserialization callsites -3. Address `normalizeToDate` fallthrough for timestamp strings -4. _(Optional cleanup)_ `new RFCDate(new Date(string))` → `new RFCDate(string)` in 3 employee form files — not a bug, but removes confusing indirection +- [ ] Extend `normalizeToISOString` to accept `Date | string | null`; delete `formatDateToStringDate` and `normalizeDateToLocal`; replace serialization callsites (see table above) +- [ ] Fix `new Date(YYYY-MM-DD)` deserialization in `PayPeriod.fromPayPeriodData`, compensation sorting, and state taxes +- [ ] Address `normalizeToDate` fallthrough for ISO timestamp strings +- [ ] _(Optional)_ Replace `new RFCDate(new Date(string))` → `new RFCDate(string)` in `useEmployeeDetailsForm`, `useHomeAddressForm`, `useWorkAddressForm` — not a bug, removes confusing indirection --- -## Phase 2: `CalendarDate` as the Internal Date Type +## Phase 2: `ISODate` as the Internal Date Type -Phase 1 fixes current bugs by convention. Phase 2 makes the correct behavior impossible to violate: `CalendarDate` has no time component and no UTC methods, so the class of bug cannot arise. +Phase 1 fixes current bugs by convention. Phase 2 introduces a lightweight internal `ISODate` class that stores a `YYYY-MM-DD` string and acts as a translation layer between date formats — providing safe construction from strings, `Date` objects, and `CalendarDate`, and converting back to any of those formats or to `RFCDate` for API calls. When arithmetic is needed (e.g. adding days or months), callsites go through `toCalendarDate()`: `isoDate.toCalendarDate().add({ days: n }).toString()`. `ISODate` itself does not perform arithmetic. -### Adapter Boundary Is Fixed +### Adapter Boundary -`DatePickerProps` and `DateRangePickerProps` are the partner-facing API and must keep `Date` at the boundary. Everything above this boundary is internal and can change. +`DatePickerProps` and `DateRangePickerProps` are the partner-facing API. Phase 2 keeps `Date` at the boundary to preserve zero breaking changes. Phase 3 replaces `Date` with `string` at this boundary — see Phase 3 for details. -Conversions happen at two edges: +`CalendarDate` from `@internationalized/date` remains strictly inside the picker components as the format required by react-aria. It does not appear in form state, models, or business logic. The picker adapter converts at two edges, unchanged from Phase 1: -- **Inbound** (adapter → form): `value.getFullYear/getMonth/getDate` → `new CalendarDate(...)` — reads local, safe -- **Outbound** (form → adapter): `new Date(cd.year, cd.month - 1, cd.day)` — local midnight, safe -- **Display**: `cd.toDate(getLocalTimeZone()).toLocaleDateString(...)` +- **Inbound** (props → react-aria): `value.getFullYear/getMonth/getDate` → `new CalendarDate(...)` — reads local, safe +- **Outbound** (react-aria → props): `new Date(cd.year, cd.month - 1, cd.day)` — local midnight, safe + +`DatePickerField` bridges the `Date` boundary and the `ISODate` form state: + +- **Inbound** (picker `onChange` → form state): `ISODate.fromDate(date)` +- **Outbound** (form state → picker `value`): `isoDate.toLocalDate()` + +### `ISODate` Class + +`ISODate` is a private SDK-internal class — not exported. It wraps a `YYYY-MM-DD` string and provides a controlled API that makes the unsafe operations impossible: + +```ts +class ISODate { + private constructor(private readonly value: string) {} + + static from(value: string): ISODate // validates YYYY-MM-DD format, throws on invalid input + static fromDate(date: Date): ISODate // safe conversion via local getters (no UTC) + static fromCalendarDate(date: CalendarDate): ISODate + + static today(): ISODate // today in local timezone + + toString(): string // returns YYYY-MM-DD + toCalendarDate(): CalendarDate // parseDate(this.value) + toLocalDate(): Date // toCalendarDate().toDate(getLocalTimeZone()) + toRFCDate(): RFCDate // new RFCDate(this.value) — direct, no Date intermediary + + isAfter(other: ISODate): boolean // lexicographic string compare + isBefore(other: ISODate): boolean + equals(other: ISODate): boolean +} +``` + +`toRFCDate()` is the key ergonomic benefit: it eliminates the `new RFCDate(new Date(str))` anti-pattern (which is both redundant and potentially buggy) that appears across the codebase. The correct path becomes `isoDate.toRFCDate()`. + +Zod schemas gain `.transform(ISODate.from)` where date fields flow through validation, making form state `ISODate | null` automatically. ### What Changes -**`PayPeriod` model:** `parseDate(data.start_date)` → `CalendarDate` instead of `new Date(data.start_date)` (UTC midnight). +**`PayPeriod` model:** Store `data.start_date` as `ISODate.from(data.start_date)` instead of `new Date(data.start_date)` (UTC midnight). -**Compensation sorting:** `parseDate(a.effectiveDate).compare(parseDate(b.effectiveDate))` instead of `new Date(effectiveDate).getTime()`. +**Compensation sorting:** `a.effectiveDate.toString() <= b.effectiveDate.toString()` — ISO strings sort lexicographically — or `a.effectiveDate.isBefore(b.effectiveDate)`. -**Address pending-change detection:** `parseDate(raw).compare(today(getLocalTimeZone())) > 0` replaces `normalizeToDate` + `startOfLocalDay`. +**Address pending-change detection:** `data.effectiveDate.isAfter(ISODate.today())` replaces `normalizeToDate` + `startOfLocalDay`. -**`useDateRangeFilter`:** State becomes `CalendarDate | null`; arithmetic via `.add({ months: n })`; serialization via `.toString()`. +**`useDateRangeFilter`:** State becomes `ISODate | null`. Arithmetic goes through `toCalendarDate()`: `ISODate.fromCalendarDate(isoDate.toCalendarDate().add({ months: n }))` when storing back as state, or `.toString()` when only the string is needed. -**State taxes date fields:** Deserialization via `parseDate(trimmed)`, serialization via `formValue.toString()`. +**State taxes date fields:** Zod schemas gain `.transform(ISODate.from)`. Deserialization changes from `new Date(trimmed)` to `ISODate.from(trimmed)`. + +**`new RFCDate(...)` callsites:** All `new RFCDate(dateValue)` and `new RFCDate(new Date(str))` callsites become `isoDate.toRFCDate()`. **`canCancelPayroll`** — the current implementation must be investigated before this migration. The hand-rolled DST logic shifts both `now` and `deadline` into a "fake PT" coordinate system (subtracting 7 or 8 hours), then uses `.setUTCHours(16, 0, 0, 0)` to pin the cutoff to 4 PM in that shifted system. This is self-consistent in the common case but has two edge-case failure modes: the DST boundary is computed at midnight local (not 2 AM), and if `now` and `payrollDeadline` straddle a DST transition they get different offsets, producing a 1-hour error. Once the intended semantics and failure modes are confirmed against tests and the API contract, the correct replacement is: @@ -199,29 +234,96 @@ Note: `deadlinePT.set({ hour: 16 })` sets 4 PM on the deadline's _date in PT_. T ### What Gets Removed -- `normalizeDateToLocal` — already removed in Phase 1 -- `formatDateToStringDate` — already removed in Phase 1 -- `normalizeToISOString` — replaced by `parseDate(s).toString()` -- `getPacificTimeOffset` — replaced by `ZonedDateTime` -- `addDays`, `isWeekend` — replaced by `.add({ days: n })` and `dayOfWeek >= 6` +- `normalizeToISOString` — replaced by `ISODate.fromDate(date).toString()` or `ISODate.fromDate(date)` depending on context +- `getPacificTimeOffset` — replaced by `now` / `parseAbsolute` from `@internationalized/date` +- `addDays`, `isWeekend` — `addBusinessDays` uses both internally; update `addBusinessDays` to use `@internationalized/date` before removing these helpers - `startOfLocalDay` (duplicated in two address files) - Duplicated `dateToCalendarDate` / `calendarDateValueToDate` in both picker files ### Risks -**1. Form value type change could be breaking.** If `DatePickerField`'s generic changes from `Date | null` to `CalendarDate | null`, partners reading form values directly would see a type change. _Mitigation: use `CalendarDate` as a safe intermediate, keep `Date` stored in form state — fixes all bugs with zero breaking changes._ +**Large migration surface.** Touches nearly every date-handling file. Requires test coverage before migrating each area. `isStringMode` and `DatePickerField`'s `TValue` generic are untouched — those are Phase 3 concerns. + +### Implementation Order + +- [ ] Implement `ISODate` class +- [ ] Replace `new RFCDate(...)` callsites with `isoDate.toRFCDate()` +- [ ] Migrate `PayPeriod` model +- [ ] Migrate compensation sorting +- [ ] Migrate address utilities (`startOfLocalDay`, pending-change detection) +- [ ] Migrate `useDateRangeFilter` +- [ ] Migrate state taxes date fields +- [ ] Investigate `canCancelPayroll` DST edge cases; migrate after semantics are confirmed +- [ ] Remove `addDays`, `isWeekend`, `normalizeToISOString`, `getPacificTimeOffset`; deduplicate picker bridge functions + +--- + +## Phase 3: `string` as the Partner-Facing Date Type (Breaking Change) -**2. `dayOfWeek` indexing differs.** `CalendarDate.dayOfWeek`: Mon=1, Sun=7. `Date.getDay()`: Sun=0, Sat=6. A direct port of `isWeekend` gets this wrong. _Mitigation: a single `isCalendarWeekend` utility._ +Phase 2 adopts `ISODate` as the internal date type. Phase 3 closes the partner-facing surface by removing `Date` from all public APIs. The external contract is plain `YYYY-MM-DD` strings — the same format the Gusto API already uses — so for the common case partners pass API strings directly with no conversion. -**3. Large migration surface.** Touches nearly every date-handling file. Requires test coverage before migrating each area. +### Motivation -### Open Question: Where Does `CalendarDate` Live in Form State? +The Gusto API returns dates as `YYYY-MM-DD` strings. The current `DatePickerProps` API requires `Date`, so partners must convert — and the obvious conversion, `new Date(apiString)`, parses as UTC midnight and produces the wrong calendar date for UTC+ users. A partner following the natural integration path introduces a UTC bug without doing anything unusual. -- **Conservative:** use `CalendarDate` as a safe intermediate only; keep `Date` in form state. Fixes all bugs, zero breaking changes, `isStringMode` survives unchanged. -- **Progressive:** store `CalendarDate` in form state. Eliminates `isStringMode`, cleaner long-term, but is technically a breaking change. +`Date` as a boundary type has no mechanism to distinguish a safely-constructed `Date` (local midnight, via `new Date(year, month-1, day)`) from an unsafe one (UTC midnight, via `new Date(string)`). The two look identical from the outside. Switching to `YYYY-MM-DD` strings eliminates the conversion step entirely: partners pass the API string directly, and the unsafe construction path does not exist. + +### What Changes + +**`DatePickerProps`:** + +| Prop | Before | After | +| --- | --- | --- | +| `value` | `Date \| null` | `string \| null` | +| `onChange` | `(value: Date \| null) => void` | `(value: string \| null) => void` | +| `minDate` | `Date` | `string` | +| `maxDate` | `Date` | `string` | +| `isDateDisabled` | `(date: Date) => boolean` | `(date: string) => boolean` | + +**`DateRangePickerProps`:** + +| Prop | Before | After | +| --- | --- | --- | +| `value` | `{ start: Date, end: Date } \| null` | `{ start: string, end: string } \| null` | +| `onChange` | `(range: { start: Date, end: Date } \| null) => void` | `(range: { start: string, end: string } \| null) => void` | +| `minValue` | `Date` | `string` | +| `maxValue` | `Date` | `string` | + +**Form `defaultValues`:** Date fields in hook `defaultValues` change from `Date` to `string`. + +**`DatePickerField` and `isStringMode`:** `DatePickerField` is currently generic over `TValue extends Date | null | string | null`. Phase 3 removes the generic — the type is always `string | null`. `isStringMode` is removed. + +### Adapter Simplification + +In Phase 2 the picker adapter is unchanged — it still converts `Date ↔ CalendarDate` using local getters. Phase 3 switches the partner boundary to `string` and introduces `ISODate` into the picker itself: + +| | Before Phase 3 | Phase 3 | +| --- | --- | --- | +| Inbound (props → react-aria) | `Date` → local getters → `CalendarDate` | `ISODate.from(value)` → `isoDate.toCalendarDate()` | +| Outbound (react-aria → props) | `new Date(cd.year, cd.month - 1, cd.day)` | `cd.toString()` | + +The last timezone-sensitive operation in the adapter layer is removed in Phase 3. + +### Format Contract + +The accepted format is `YYYY-MM-DD`. Strings with time components are not valid input. The SDK should warn in dev mode if a non-date string is passed. As a belt-and-suspenders measure, internal parsing can guard with `.split('T')[0]` when consuming partner-supplied strings. + +### Partner Utilities + +The partner-facing type is plain `string`. When passing values from the API directly, no conversion is needed. For cases where a string must be constructed (e.g. `minDate`, `defaultValues` from a `Date` the partner already has), the SDK exports an `ISOString` namespace: + +```ts +ISOString.today(): string // today in local timezone — replaces minDate={new Date()} +ISOString.from(date: Date): string // safe Date → YYYY-MM-DD using local getters +ISOString.isValid(s: string): boolean +``` -`isStringMode` exists because `DatePickerField` is generic over `Date | null` _or_ `string | null`. Partners that bind date fields to a Zod schema where values round-trip as strings rely on the string output path. Before taking the progressive path, confirm no partner uses `DatePickerField` — check SDK documentation and partner-facing examples. +`ISOString` is a plain object — not a class, not a type. Partners import it like any other utility. The internal `ISODate` class is not exported. ### Implementation Order -`DatePickerField` → `PayPeriod` → address utilities → `canCancelPayroll` (after bug investigation) → remaining form files +- [ ] Update `DatePickerProps` and `DateRangePickerProps` type signatures +- [ ] Update adapter conversions in `DatePicker.tsx` and `DateRangePicker.tsx` — inbound `ISODate.from(value)` → `isoDate.toCalendarDate()`, outbound `isoDate.toString()` +- [ ] Remove `TValue` generic and `isStringMode` from `DatePickerField` +- [ ] Update all exported hook `defaultValues` types for date fields +- [ ] Export `ISOString` namespace