Skip to content

fix(SDK-810): emit local calendar dates in DatePickerField string mode#1630

Open
hukid wants to merge 1 commit intomainfrom
fix/sdk-810-datepicker-tz-shift
Open

fix(SDK-810): emit local calendar dates in DatePickerField string mode#1630
hukid wants to merge 1 commit intomainfrom
fix/sdk-810-datepicker-tz-shift

Conversation

@hukid
Copy link
Copy Markdown
Contributor

@hukid hukid commented Apr 28, 2026

Summary

  • Fix formatDateToStringDate and normalizeDateToLocal to read local-time year / month / day instead of routing through toISOString(). In UTC+ timezones the UTC representation of a local-midnight Date sits on the previous calendar day, so the old implementations shifted the result back by a day each.
  • Without this fix, DatePickerField in isStringMode chains both helpers, so the form value (and the date sent to the API) was two days behind what the user picked in calendars like UTC+5:30 (IST) or UTC+14 (Kiritimati).
  • Pin the behavior with a parameterized test suite that re-runs the helper assertions in UTC, UTC-8 (PST), UTC+5:30 (IST), and UTC+14 (Kiritimati), including a round-trip test that mirrors what DatePickerField does in string mode.

Why

The codebase already knows about this pitfall in two places — they just hadn't been generalized:

  • src/helpers/dateFormatting.ts already had normalizeToISOString, with this docstring:

    Unlike formatDateToStringDate (which reads UTC), this function reads the local year/month/day — safe for dates parsed from locale-format strings... or ISO strings, both of which normalizeToDate lands at local midnight.

  • src/components/Common/UI/DatePicker/DatePicker.tsx has a private dateToCalendarDate helper (lines 29-42) with this comment:

    // Use local date parts to avoid UTC timezone shift from toISOString()

This PR brings the two general-purpose helpers in line with that already-documented pattern.

Bug trace (UTC+5:30, our react-aria adapter)

  1. User clicks April 16 in the calendar.
  2. react-aria emits DateValue { year: 2024, month: 4, day: 16 }.
  3. calendarDateValueToDate converts to new Date(2024, 3, 16) = April 16 local midnight (= April 15 18:30 UTC).
  4. DatePickerField.handleChange:
    1. normalizeDateToLocal(value) calls value.toISOString()"2024-04-15T18:30:00.000Z" → builds new Date(2024, 3, 15) = April 15 local midnight (one day off).
    2. formatDateToStringDate(...) calls .toISOString() again on April 15 local → "2024-04-14T18:30:00.000Z" → returns "2024-04-14" (two days off).

The form ends up with "2024-04-14". After this PR, the chain returns "2024-04-16" in every timezone tested.

What changed

  • src/helpers/dateFormatting.ts
    • formatDateToStringDate now builds the YYYY-MM-DD string from getFullYear / getMonth / getDate (with zero-padding) instead of toISOString().split('T')[0].
    • normalizeDateToLocal now returns new Date(date.getFullYear(), date.getMonth(), date.getDate()) instead of round-tripping through toISOString().
    • Added doc comments explaining the local-time contract and the UTC pitfall.
  • src/helpers/dateFormatting.test.ts
    • Replaced the two pre-existing normalizeDateToLocal tests that asserted UTC-input behavior with locale-deterministic equivalents (the old assertions were only correct in UTC and would fail elsewhere).
    • Added a describe.each block running in UTC, PST, IST, and UTC+14, exercising:
      1. formatDateToStringDate against local-midnight, late-evening, and early-morning Dates.
      2. normalizeDateToLocal no-op behavior on local-midnight Dates and time-zeroing on Dates with a time component.
      3. The string-mode round-trip (normalizeDateToLocalformatDateToStringDate) preserving the user-picked calendar date.

Behavior change for partner adapters

The new normalizeDateToLocal interprets its input in local time. For our default react-aria adapter (which already passes local-midnight Dates), this is a no-op and the user-visible bug is fixed. For a hypothetical partner adapter that passes UTC-midnight Dates, the result in UTC- timezones now shifts to the previous local calendar day (since UTC midnight there is previous-day 16:00 local). This is consistent with the rest of the date pipeline now reading local — and the prior behavior was already broken for the much more common local-midnight adapter case in UTC+ timezones, so optimizing for the actually-shipped adapter is the right trade-off.

Test plan

Automated:

  • New parameterized timezone tests cover the helpers and the round-trip in 4 timezones.
  • npm run test -- --run passes (2,340 / 2,340).
  • npm run tsc clean.
  • ESLint clean on changed files.

Manual:

  • Run TZ="Asia/Kolkata" npm run storybook and pick a future date in any DatePickerField story (e.g. Forms/DatePickerField). Confirm the field's stored value matches what was clicked.
  • Repeat in the SDK dev app for any flow that uses a DatePickerField (e.g. employee start date) and verify the date the API receives matches the picked date.

Made with Cursor

`formatDateToStringDate` and `normalizeDateToLocal` both extracted year /
month / day via `toISOString()`, which returns the UTC interpretation of
the Date. For users in UTC+ timezones the local calendar date sits before
UTC midnight on the previous day, so each helper shifted the result back
by one day. `DatePickerField` chains both helpers in string mode, so the
form value (and the date sent to the API) ended up two days behind what
the user picked in calendars like UTC+5:30 or UTC+14.

Switch both helpers to local-time getters (`getFullYear`, `getMonth`,
`getDate`) so they describe the date the user actually sees. This matches
the existing `normalizeToISOString` helper and the inline `dateToCalendarDate`
in `DatePicker.tsx`, both of which already deliberately avoid `toISOString()`
for the same reason.

Add a parameterized timezone test suite that pins behavior across UTC,
UTC-8 (PST), UTC+5:30 (IST), and UTC+14 (Kiritimati) for both helpers and
the DatePickerField round-trip.

Made-with: Cursor
Copilot AI review requested due to automatic review settings April 28, 2026 23:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes DatePickerField string mode producing off-by-one/two-day calendar dates in UTC+ timezones by making date normalization/formatting read local year/month/day rather than routing through toISOString().

Changes:

  • Update formatDateToStringDate to build YYYY-MM-DD from local date parts.
  • Update normalizeDateToLocal to return a new local-midnight Date using local date parts.
  • Add parameterized tests intended to pin helper behavior across multiple timezones and a string-mode round trip.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/helpers/dateFormatting.ts Switches string-date formatting and local-midnight normalization to use local calendar parts; adds documentation.
src/helpers/dateFormatting.test.ts Updates existing normalization tests and adds timezone-parameterized coverage plus a round-trip assertion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +171 to +179
/**
* Formats a Date as a `YYYY-MM-DD` string using the date's *local* calendar
* date (year/month/day), so the output reflects the calendar date the user
* actually picked or stored regardless of the runtime timezone.
*
* Reading via `toISOString()` would shift the date by a day for users in
* UTC+ timezones, where local midnight falls before UTC midnight on the
* previous calendar day.
*/
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatDateToStringDate now explicitly uses local date parts, but the nearby normalizeToISOString docstring later in this file still claims it is "Unlike formatDateToStringDate (which reads UTC)". Please update that docstring to reflect the new local-time behavior so consumers aren’t misled about which helper is UTC vs local.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good catch, this is what i was noodling on yesterday. trying to figure out if this formatDateTOStringDate function should be preserved as is for a distinct use case. If we are updating this as seen in this PR, it should likely be reconciled with normalizeToISOString

})

afterAll(() => {
process.env.TZ = originalTZ
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

afterAll restores process.env.TZ by assignment, but if originalTZ was undefined this will set the env var to the literal string "undefined" and can leak a bogus timezone into later tests. Prefer restoring with delete process.env.TZ when originalTZ is not set, otherwise reassign the saved value.

Suggested change
process.env.TZ = originalTZ
if (originalTZ === undefined) {
delete process.env.TZ
} else {
process.env.TZ = originalTZ
}

Copilot uses AI. Check for mistakes.
Comment on lines +231 to +233
{ tz: 'America/Los_Angeles', label: 'UTC-8 (PST)' },
{ tz: 'Asia/Kolkata', label: 'UTC+5:30 (IST)' },
{ tz: 'Pacific/Kiritimati', label: 'UTC+14' },
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The label "UTC-8 (PST)" for America/Los_Angeles is not always correct due to DST (it can be UTC-7). Consider renaming the label to something timezone-name-based (e.g., "America/Los_Angeles" or "Pacific Time") to avoid misleading test output.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@serikjensen serikjensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a question in the channel around this one, i think we need to get a little more clarity around our expected date behavior https://gustohq.slack.com/archives/C071WFJUNF5/p1777474103283499f

Comment on lines +171 to +179
/**
* Formats a Date as a `YYYY-MM-DD` string using the date's *local* calendar
* date (year/month/day), so the output reflects the calendar date the user
* actually picked or stored regardless of the runtime timezone.
*
* Reading via `toISOString()` would shift the date by a day for users in
* UTC+ timezones, where local midnight falls before UTC midnight on the
* previous calendar day.
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good catch, this is what i was noodling on yesterday. trying to figure out if this formatDateTOStringDate function should be preserved as is for a distinct use case. If we are updating this as seen in this PR, it should likely be reconciled with normalizeToISOString

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants