[pickers] Fix DST regression in AdapterDayjs.adjustOffset#22278
Draft
LukasTy wants to merge 4 commits intomui:masterfrom
Draft
[pickers] Fix DST regression in AdapterDayjs.adjustOffset#22278LukasTy wants to merge 4 commits intomui:masterfrom
AdapterDayjs.adjustOffset#22278LukasTy wants to merge 4 commits intomui:masterfrom
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>
LukasTy
added
type: regression
Deploy previewhttps://deploy-preview-22278--material-ui-x.netlify.app/ Bundle size
Check out the code infra dashboard for more information about this PR. |
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #21669.
Summary
In a non-UTC system timezone (e.g. Los Angeles) on a DST start day (e.g. March 8, 2026), picking a time like
04:00 AMin the time picker silently produced a different underlying instant. v7 was fine; v8/v9 regressed.Root cause
AdapterDayjs.adjustOffsetmutatedvalue.$offsetafter everysetX/addX/startOfX/endOfX. This was correct for tz-aware dayjs values (where$dis local-as-UTC and$offsetcompensates) but wrong for plaindayjs()values, whosevalueOf()is computed as:Mutating only
$offset(without updating$d) drifted the underlying instant by the guessed-zone offset. Display getters (hour(),format()) still looked right because they read$ddirectly, which is why this slipped through.PR #22170 (April 2026) simplified
createSystemDateto return plaindayjs(value), which is exactly the path where the mutation produces wrong results.What changed
adjustOffsetnow early-returns for plain dayjs and UTC values, and uses a cleanvalue.tz(timezone, true)round-trip for tz-aware values. No more direct$offsetmutation.adjustOffsetwrappers from the eightstartOfX/endOfXmethods. The dayjs timezone plugin'sstartOfoverride already handles DST for tz-aware values, JS Date semantics handle it for plain values, andendOfis implemented asstartOf(unit, false)in dayjs core, so it inherits the same handling.adjustOffseton the sevenaddXand sevensetXmethods (the dayjs timezone plugin does not override.add()or.set(), so explicit DST handling is still needed there).#13290 stays fixed: plain values are returned untouched, so no spurious
$x.$timezoneis attached.Tests
New regression tests in
AdapterDayjs.test.tsx:should not mutate $offset on plain dayjs() values when dayjs.tz.guess() is non-UTC (regression #21669)- asserts the invariant directly via$offsetandtoDate().toISOString().addMonths.Time picker (regression #21669) > should call onChange with the clicked hour when picking on a DST start day with system timezone- end-to-end via<DigitalClock timezone=\"system\">, mirrors the user's reproduction.New shared "should update the offset when entering DST" cases for
setDateandsetHoursindescribeGregorianAdapter/testCalculations.ts, mirroring the existingaddMonths/addWeeks/addDayscoverage.Sanity-checked by temporarily reverting
adjustOffsetto the old mutation: all three new dayjs tests fail with the buggy code and pass with the fix; every other test passes in both states.Test plan
pnpm test:unit --project \"x-date-pickers\" --runpnpm test:unit --project \"x-date-pickers-pro\" --runpnpm test:browser --project \"x-date-pickers\" --runpnpm --filter \"@mui/x-date-pickers*\" run typescriptpnpm prettierandpnpm eslint04:00 AMin the time pickerdayjswhen timezone extensions are enabled #13290 - DateRangePicker keyboard date change with utc/timezone plugins enabled