Repository navigation
[pickers] Fix Day.js named-zone elapsed-time additions across DST - #23752
Conversation
Replace the `value.$offset` mutation with a `value.tz(tz, true)` round-trip for tz-aware values, and skip the adjustment for plain `dayjs()` values. The mutation corrupted plain values' `valueOf()` because their formula relies on `$offset` and `$d.getTimezoneOffset()` together. After PR mui#22170 made system-timezone values plain, this surfaced as the time picker silently shifting the picked hour on DST start days in non-UTC system zones. Drop the redundant `adjustOffset` from `startOfX`/`endOfX` - the dayjs timezone plugin already handles DST inside its `startOf` override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The first cut at the fix replaced the `$offset` mutation with
`return value.tz(timezone, true)`, which has two side effects:
- the timezone plugin's "keep local time" branch runs
`n.add(i - m, 'minute')`, shifting `$d` whenever the offset changes;
- dayjs's `utcOffset(_, true)` does not write `$x.$localOffset`,
leaving the side table that `valueOf()` consults empty.
For tz-aware values built via `dayjs.tz(...)` (which sets `$localOffset`
via the no-keep-local-time branch of `utcOffset`), this combination made
`$d.getHours()` land on the wrong side of the DST gap once the system
timezone was non-UTC (e.g. user's LA env on March 8). The picker reports
`getHours(setHours(value, X))` against `X` to decide whether each hour
slot is selectable, so the off-by-one shift visibly disabled hours
adjacent to the gap (1 disabled slot turned into 3).
Restoring the in-place `$offset` mutation - while keeping the
plain-value early return that fixes the original mui#21669 - leaves `$d`
and `$x.$localOffset` untouched, which is what `valueOf()` and
`getHours()` rely on.
Adds a structural test that pins both invariants for the `dayjs.tz(...)`
construction path; the test fails with the `value.tz(timezone, true)`
return and passes with the mutation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous early-return check was `if (!value.$x?.$timezone)`, which also skipped values that came through `dayjs(...).tz(undefined, false)` - the picker's `now` from `createTZDate(undefined, 'default')`. Those values have `$offset` set to the system zone's construction-time offset but no `$x.$timezone`. Skipping `adjustOffset` left `$offset` stale after `setHours` to a different DST regime, so the picker's `onChange` handed the parent a value whose underlying instant drifted an hour from the picked clock time, even though `getHours()` looked right. Tighten the early return to check `$offset` instead - only pure `dayjs(value)` values (no `$offset`) skip the mutation, while anything that has been touched by `.tz()` runs through the OLD mutation logic. This restores the v8/v9 behavior for the picker's `now` value while keeping the mui#21669 fix for plain values and the mui#13290 fix (plain values stay plain, no spurious timezone metadata). Restores the `adjustOffset` wrappers on `startOfX`/`endOfX` for the same reason - the picker chains those with `setX`/`addX` and any drift in the start/end value would propagate. Adds a regression test that pins the `now`-shape behavior. Sanity- checked: the test fails with the over-aggressive early return and passes with the tightened one. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`(value.set('hour', hour) as Dayjs).$d` doesn't typecheck - `$d` is a
private dayjs internal not on the public `Dayjs` type, and the cast is
to the same type that lacks it. Moving the existing `// @ts-ignore`
comment one line up covers the call.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…kilby-9a1521 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Skip only plain `system` values in `adjustOffset`. - Remove a stale `$x.$localOffset` before the offset is recomputed. - Return the recomputed value when a zone moves to or from offset 0. - Replace the stubbed `dayjs.tz.guess()` tests with tests that switch `process.env.TZ` at runtime. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kilby-9a1521 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kilby-9a1521 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> # Conflicts: # packages/x-date-pickers/src/AdapterDayjs/AdapterDayjs.test.tsx
- Keep `$x.$localOffset` after hour, minute and second additions. - Restore the equivalent-offset guard, which dayjs < 1.11.12 needs. - Rebuild the value with `dayjs.tz` when a zone moves to or from offset 0, so that dayjs < 1.11.12 keeps the wall clock and locale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Clear `$x.$localOffset` only after calendar edits, and only when it matched the source value before the edit. - Remove the rebuild for zones that move to or from offset 0. It lost elapsed time and broke small years, large years and milliseconds. - Pass the source value from `AdapterDayjsBuddhist.setYear` too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Remove the elapsed-time and unchanged-setter tests. They guarded named-zone logic that this PR no longer contains. - Test small years with a plain value, which this PR fixes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LukasTy
added
type: bug
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
…-elapsed-dst # Conflicts: # packages/x-date-pickers/src/AdapterDayjs/AdapterDayjs.test.tsx
- Store `$x.$localOffset` only when `valueOf()` needs it. Later setters copy it, so a stored offset made them 1 hour wrong. - Build the first instant of the day in `startOfDay` for named zones, so that DigitalClock lists every option on DST days. - Read named-zone offsets in `adjustOffset` from Intl, and rebuild values that leave or reach offset 0. - Clone the value before `tz()`, because dayjs 1.11.12 and 1.11.13 change the value itself. - Keep the previous behavior on dayjs 1.10.7 to 1.11.1, where `valueOf()` uses the system offset of today. - Read offsets from numeric Intl parts, because Node 14 and 16 do not support `timeZoneName: 'longOffset'`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-elapsed-dst Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LukasTy
added
needs cherry-pick
How to testThe changes are committed and pushed at 3518a6f. The demo compares named-zone Day.js values in two columns. Master after #22278 reproduces the affected methods from Set the browser timezone
The browser timezone and the value's named timezone are separate. Each example constructs its value with Main regression: select both repeated-hour optionsKeep New York · repeated hour selected. The controlled value starts at
The master column emits Other clock examplesUse Clock example to switch scenarios. With the New York browser timezone, expect these results in the PR column:
The arithmetic section should show PR: 36/36 OK. Try each addition method in both directions, or expand all checks. These also check wall fields, offsets, milliseconds, locale, cloning, and source immutability. Both later-setter examples should be OK. Repeat with browser timezone Known limitationSet the browser timezone to |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The implementation relies on Day.js internals and cross-version timezone behavior that warrants final human review.
Review effort: Balanced
Findings: None
What changed in this PR
Fixes Day.js named-timezone arithmetic across DST transitions while preserving locale, precision, and timezone identity.
Changes:
- Rebuilds named-zone values from target instants and accurate
Intloffsets. - Corrects named-zone
startOfDaybehavior around DST gaps and overlaps. - Adds extensive adapter and DigitalClock regression coverage.
| File | Description |
|---|---|
AdapterDayjs.ts |
Implements DST-safe elapsed arithmetic and offset adjustment. |
AdapterDayjs.test.tsx |
Covers DST, historical offsets, precision, and immutability. |
timezone.DigitalClock.test.tsx |
Expects both repeated-hour options for every adapter. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
JCQuintas
left a comment
JCQuintas
left a comment
There was a problem hiding this comment.
claude found some issues, I made him create examples of the issues, while some aren't actually an issue of this PR (but probably should be solved), there is one that is.
| private addTime = (value: Dayjs, amount: number, unit: 'hour' | 'minute' | 'second') => { | ||
| // @ts-ignore | ||
| const timezone = value.$x?.$timezone; | ||
| if (!this.hasTimezonePlugin() || !timezone || !this.hasValueOfDSTFix()) { |
There was a problem hiding this comment.
#1 Default-timezone values still repeat the 1 AM hour
This path only runs when $x.$timezone is set. Values created for the default timezone when the timezone plugin is loaded (adapter.date(undefined, 'default'), i.e. dayjs().tz(undefined), or dayjs().tz() in user code) have an $offset but no $x.$timezone. getTimezone() returns 'system', so they still go through adjustOffset(value.add(...)), which keeps the wall time and picks the first matching offset.
System TZ America/New_York, DigitalClock with timeStep={30} on 2026-11-01:
12:00 AM 04:00Z
12:30 AM 04:30Z
01:00 AM 05:00Z
01:30 AM 05:30Z
01:00 AM 05:00Z <- same instant as row 3
01:30 AM 05:30Z <- same instant as row 4
02:00 AM 07:00Z
50 options, 48 distinct instants: 06:00Z and 06:30Z can't be selected, and React warns about duplicate keys. Master gives the same output. This is the uncontrolled-picker setup with the plugin loaded, so probably the most common one.
Could values with an offset but no zone name use dayjs.tz.guess() as their zone here (and in startOfDay / adjustOffset)? Otherwise, it's worth stating in the description that only values with a named zone are covered.
There was a problem hiding this comment.
Covered in dfd5650. addHours/addMinutes/addSeconds now add to the instant and convert back with setTimezone(), so values with an offset but no timezone name come back as plain system values (getTimezone() stays 'system'). startOfDay uses the native Date for them. Your example now gives 50 options and 50 distinct instants. One case stays as on master: at offset 0 (Europe/London in winter), tz() gives a UTC value that the adapter cannot tell from dayjs.utc(). The description lists it.
| const localDate = new Date(0); | ||
| localDate.setFullYear( | ||
| wallDate.getUTCFullYear(), | ||
| wallDate.getUTCMonth(), | ||
| wallDate.getUTCDate(), | ||
| ); | ||
| localDate.setHours( | ||
| wallDate.getUTCHours(), | ||
| wallDate.getUTCMinutes(), | ||
| wallDate.getUTCSeconds(), | ||
| wallDate.getUTCMilliseconds(), | ||
| ); | ||
| result = dayjs(localDate).locale(locale); |
There was a problem hiding this comment.
#2 Wall fields are wrong when the target wall time falls in a system DST gap
The description lists this as a known limitation with a New York value in a London system, but it's more common than that: all EU zones switch at 01:00Z, so any two different EU zones hit it on the last Sunday of March.
System TZ Europe/London, value dayjs.tz('2026-03-29T00:00', 'Europe/Paris'), DigitalClock with timeStep={30}:
label instant actual Paris time
00:00 2026-03-28 23:00Z 00:00
00:30 2026-03-28 23:30Z 00:30
02:00 2026-03-29 00:00Z 01:00 <- wrong label
02:30 2026-03-29 00:30Z 01:30 <- wrong label
03:00 2026-03-29 01:00Z 03:00
Picking "02:00" emits 00:00Z (01:00 in Paris). After that, setDate(value, 29), which should be a no-op, returns 01:00Z (+1 h), because the setter rebuilds from the shifted wall fields.
Not a regression: master shows the same wrong labels and also repeats an instant. I agree a real fix needs a different representation, so I'd only update the limitation note so it doesn't read like an edge case.
There was a problem hiding this comment.
Updated the limitation in the description with your example. It now says that any two EU timezones with different offsets hit it on the last Sunday of March. With the simplified implementation, tz() also gets the instant wrong in that hour, as on master, and the description says so.
| if (this.hasTimezonePlugin() && timezone && this.hasValueOfDSTFix() && value.isValid()) { | ||
| return this.getStartOfDayInTimezone(value, timezone); | ||
| } |
There was a problem hiding this comment.
#3 Regression: 12-hour fields only accept 01 in the hours section on days where midnight is skipped
Returning the first instant that exists is right (Luxon and Moment also return 01:00 here), but getSectionsBoundaries formats startOfDay(today) to find out how midnight is written:
const formattedMidnight = Number(
removeLocalizedDigits(adapter.formatByString(adapter.startOfDay(today), format), localizedDigits),
);
// ...
// h/hh format (hour 1-12): midnight formats as 12
return { minimum: 1, maximum: formattedMidnight };With timezone="America/Santiago" on 2026-09-06, startOfDay(today) is now 01:00 -03:00, so for hh the hours range becomes 1 to 1 (master: 00:00 -03:00, which formats as 12, so 1 to 12). Typing 3 or 10 in the hours section is ignored, and ArrowUp/ArrowDown stay on 01, for the whole day. Same for America/Havana (Mar 8), Atlantic/Azores and Asia/Beirut (Mar 29) and Africa/Cairo (Apr 24) in 2026. HH formats are fine.
Since AdapterLuxon and AdapterMoment already return 01:00 there, they should already hit this. I'd fix it in getSectionsBoundaries (e.g. format midnight of a fixed date whose midnight exists) rather than revert startOfDay. The StackBlitz pins "today" to 2026-09-06 to reproduce it.
There was a problem hiding this comment.
Fixed in dfd5650: getSectionsBoundaries formats midnight of 2000-01-01, which exists in all 418 IANA timezones (tzdata 2026b). The new TimeField test (America/Santiago on 2026-09-06, hh) fails before the change on Day.js, Luxon, and Moment.
michelengelen
left a comment
michelengelen
left a comment
There was a problem hiding this comment.
Just a suggestion for maintainability later on and one note about the PR description: You list very large validation counts (92,610 cases, 44,910 comparisons, 14 day.js versions) come from your own scripts that are not in the repository, right? Can you commit the matrix script (for example under scripts/ or as a skipped test), so other maintainers can repeat them?
- Rebuild AdapterDayjs on master with public Day.js APIs only. Elapsed additions add to the instant and convert back with setTimezone(). startOfDay and adjustOffset resolve wall times with tz() conversions. - Target Day.js 1.11.22 or later, and recommend it in the timezone docs. - getSectionsBoundaries formats midnight of 2000-01-01, so hh accepts every hour on a day that skips midnight. - Remove the Local Mean Time and system-gap tests. The simple version keeps the master behavior there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@michelengelen The counts came from local scripts that compared the adapter with Adding the exploratory tests would be a bit too much as it tests many day.js version to map coverage. |
|
Cherry-pick PRs will be created targeting branches: v8.x, v9.x |
AdapterDayjs.addHours,addMinutes, andaddSecondskeep the wall time of a timezone value and adjust the offset. Across a DST change, this changes the elapsed duration. With aDigitalClockvalue ofdayjs.tz('2026-11-01T12:00', 'America/New_York'),timeStep={30},ampm, and system timezoneAmerica/New_York, both 01:30 AM options emit05:30Z, and React warns about duplicate keys. Values thattz()gives without a timezone name have the same bug. An uncontrolled picker uses such values when the timezone plugin is loaded and no default timezone is set.This PR uses only the public Day.js API and targets Day.js 1.11.22 or later. Since that version,
tz()reads the offset of an instant fromIntl. The timezone docs now recommend this version, and v10 can raise the minimum version. On Day.js 1.11.22 or later,masteralready handles #21669 correctly.Follow-up to #22278.
Changes
addHours,addMinutes,addSeconds: add the amount to the instant, then convert the result back to the timezone of the value. The two 01:30 AM options now emit05:30Zand06:30Z.startOfDay: return the first instant of the day.startOf('day')can give a midnight in a DST gap (America/Santiagoon 2026-09-06) or the previous day (Europe/Londonon 2026-10-25 with a US system timezone). When midnight repeats (America/Havana,Atlantic/Azores), the first midnight starts the day.adjustOffset(setters and calendar operations): rebuild a named-zone value from its wall time.tz(timezone, true)breaks when the offset changes to or from 0: with a US system timezone,setMonth(dayjs.tz('2026-07-15T12:00', 'Europe/London'), 0)gave17:00Zinstead of12:00Z. A repeated wall time keeps its current offset when that offset is valid. A wall time in a DST gap moves forward.tz()adds a second to an instant with milliseconds. The adapter converts the whole second, then adds the milliseconds.getSectionsBoundaries: format midnight of 2000-01-01 to find how the format shows hour 0. On a day that skips midnight,startOfDayis at 01:00, sohhaccepted only01in the hours section for the whole day.AdapterLuxonandAdapterMomenthad the same bug.Validation
x-date-pickersandx-date-pickers-projsdom suites underTZ=UTC: 4,841 passed, 1,134 skipped, 2 todo. The focused adapter, clock, and field suites also pass in Chromium.DigitalClock,startOfDayon DST days, setters to and from offset 0, and the hours section on a day without midnight. Each new branch has a test that fails without it.masteron Day.js 1.11.22 and 1.11.23 (all changed methods, 7 system timezones) found no regression, except the system-gap case below. On older Day.js versions, some results near DST changes of the system timezone are worse than onmaster.Known limitations
Day.js stores the wall fields of a nonzero-offset value in a local
Date. On the day that the system timezone skips an hour, a named-zone time in that hour gets wrong wall fields, andtz()gives an instant that is 1 hour wrong. Any two EU timezones with different offsets hit this on the last Sunday of March. Example: system timezoneEurope/London, valuedayjs.tz('2026-03-29T00:00', 'Europe/Paris'):DigitalClockdoes not list 01:00 and 01:30 in Paris, and shows02:00for 03:00.masteralso shows wrong labels there, and repeats an instant. With system timezoneEurope/Berlin,TimeClockalso disables 02:00 of a London value on 2026-03-29, whichmasterenables.At offset 0,
tz()without a timezone name gives a UTC value (system timezoneEurope/Londonin winter). The adapter cannot tell it from adayjs.utc()value, so it keeps UTC arithmetic, as onmaster.Local Mean Time offsets (dates before about 1900) keep the Day.js behavior.
I have followed (at least) the PR section of the contributing guide.
Generated with Claude Code