Skip to content

fix(timezone): stop inheriting host DST in instance .tz() offset - #3169

Open
Moumouls wants to merge 2 commits into
iamkun:devfrom
Moumouls:fix/timezone-host-dst-offset-guadeloupe
Open

fix(timezone): stop inheriting host DST in instance .tz() offset#3169
Moumouls wants to merge 2 commits into
iamkun:devfrom
Moumouls:fix/timezone-host-dst-offset-guadeloupe

Conversation

@Moumouls

@Moumouls Moumouls commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes incorrect timezone offsets when converting an instant with instance .tz() on a host that observes EU-style daylight saving time.

Bug: America/Guadeloupe is a fixed UTC−4 zone (no DST). On hosts such as Europe/Paris or Europe/London, around the EU spring-forward Sunday, dayjs(instant).tz('America/Guadeloupe') could report −03:00 instead of −04:00. Adding one day and converting again could then land on the same calendar day.

Root cause: proto.tz derived the offset from:

const target = date.toLocaleString('en-US', { timeZone })
const diff = Math.round((date - new Date(target)) / 1000 / 60)
const offset = (-Math.round(date.getTimezoneOffset() / 15) * 15) - diff

Both new Date(target) and getTimezoneOffset() depend on the host timezone. When the host has already switched to DST for that instant but the wall-clock string still parses on the standard-time side of the transition, the offset for a fixed zone is wrong by one hour.

The static helper dayjs.tz(string, zone) already used tzOffset via Intl.DateTimeFormat#formatToParts (host-independent). Instance .tz() did not.

Fix: reuse the existing tzOffset(+date, timezone) helper in proto.tz for the offset. Keep wall-clock construction via toLocaleString and the rest of the method unchanged (utcOffset, keepLocalTime, $timezone).

Related: #1260

Test plan

  • Added regression tests for America/Guadeloupe around EU spring DST (2050-03-27)
  • TZ=Europe/Paris npm run test-tz-plugin — pass (including Guadeloupe cases)
  • TZ=Europe/London npm run test-tz-plugin — pass
  • Existing npm run test-tz host matrix still covers test/timezone.test

Reproduce before the fix:

TZ=Europe/Paris npx jest test/plugin/timezone.test.js -t 'America/Guadeloupe' --coverage=false

Credits

This PR was prepared with Cursor and assistance from Grok 4.5 High.

Use tzOffset (Intl formatToParts) in proto.tz instead of
toLocaleString + new Date + getTimezoneOffset, so fixed-offset
zones like America/Guadeloupe keep UTC-4 across host spring DST.
@Moumouls

Moumouls commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

I can confirm this patch on our company monorepo fix the issue

@karlhorky

karlhorky commented Aug 4, 2026

Copy link
Copy Markdown

@iamkun would you be available to review this? (or would you pass it off to another maintainer for review?)

When I speak with other engineers and our students, I always suggest avoiding Day.js, because of the surprising and severe time zone bugs. I would love to be able to start recommending the library again.

@Moumouls

Moumouls commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@karlhorky i sent an email to @iamkun with some luck it could land asap and released

@iamkun

iamkun commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Thanks a lot for tracking this down and fixing it — this is a real, long-standing pain point (see #1260 and the related issues), and the fix is clean: reusing the existing host-independent tzOffset() logic instead of patching around the old host-dependent calculation is exactly the right approach.

Before merging, could you add a few more regression cases to test/plugin/timezone.test.js?

  1. A target zone that itself observes DST, tested with host/target in different DST states at the same instant (e.g. host in DST while target has already fallen back, and vice versa). The Guadeloupe case covers "target has no DST," but the original bug was about host DST leaking into the offset calculation in general, so we should also confirm it's fixed when both zones observe DST but are out of sync.
  2. A keepLocalTime: true case — that branch depends on ins.utcOffset(), which is now produced by the new tzOffset() path. It's not directly touched by the diff, but there's no test currently pinning its behavior under the new logic.
  3. If convenient, it'd also be good to extend test-tz-plugin coverage to a non-EU host timezone (e.g. something in the southern hemisphere, or America/New_York) to confirm the fix isn't specific to the Paris/London host pair.

Once these are in, I'm happy to merge — this is a great fix and I'd like to get it into a release soon.

@Moumouls

Copy link
Copy Markdown
Contributor Author

Thanks @iamkun i'll add this asap

@Moumouls

Copy link
Copy Markdown
Contributor Author

@iamkun I’m currently using GPT-5.6 Sol xhigh to battle-test the fix, add more coverage, and fix the TZ plugin as much as possible to minimize future maintenance and bug fixes. The PR will be slightly bigger

Store timezone wall clocks in UTC and resolve DST gaps/overlaps like Moment so host timezone transitions no longer skip hours or pick the wrong offset.
@Moumouls

Copy link
Copy Markdown
Contributor Author

@iamkun It’s ready. I made multiple passes to simplify the code as much as possible and maximize both code coverage and functional coverage with Sol xhigh. I believe the TZ extension is now almost bulletproof, and it should fix many of the TZ issues in Day.js.

@Moumouls

Copy link
Copy Markdown
Contributor Author

Sorry for the added lines ( mostly to coverage edge case, like a DST shift a sunday in 2050...)

@iamkun

iamkun commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Thanks for the context. Just to make sure I understand correctly — you're planning to add more tests and battle-test the fix on top of the existing PR, right?

The additional test coverage sounds great — thank you for that. One small ask though: if this PR also ends up touching refactoring or other fixes for the TZ plugin, would it be okay to move those to a separate PR? That would really help with keeping the review manageable.

@Moumouls

Moumouls commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@iamkun final version pushed, tell me if it need a split, the only caveat, the approach is based on "moment feature parity" for coverage so if moment have a TZ bug, dayjs is likely to reproduce it.

Tell me if you need a split here, but the internal refactor to fix properly that big, i think the review is manageable, tell me if you need something or if the review is too hard to manage on this final commit ?

@Moumouls

Copy link
Copy Markdown
Contributor Author

The refactor was indeed necessary, @iamkun. Without it, fixing all the tests properly made the code quite tricky at some point. Once all the tests were passing, I did four rounds of simplification with Sol 5.6 xhigh wherever possible. I also reviewed the lines directly in Cursor and read through the code myself.

@Moumouls

Copy link
Copy Markdown
Contributor Author

reported coverage on TZ plugin is 100%

@iamkun

iamkun commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Thanks again for digging into this — and for the much deeper follow-up. A few thoughts on scope:

1. The original fix is good as-is. The minimal change to proto.tz() (reusing tzOffset() instead of the host-dependent diff calculation) is correct and low-risk. Let's add the additional regression tests we discussed (DST-observing target zones with host/target in different DST states, a keepLocalTime case, a non-EU host timezone), and get that merged and released — it fixes the reported issue and shouldn't wait on anything else.

2. The broader set/add/startOf fixes are a separate, real problem — but let's not couple them to dayjs core or the utc plugin. We agree there's a genuine need for a "wall-clock arithmetic decoupled from the host timezone" mechanism to make calendar operations (set, add on calendar units, startOf) correct across DST edges on zoned instances — that's a real bug independent of the original report, not just scope creep.

That said, we'd rather not solve it by repurposing $d to store a fake-UTC wall clock and then patching Dayjs.prototype.parse/$set plus the utc plugin's valueOf to recognize a new private sentinel ($x.$localOffset === 0). That approach works, but it couples timezone and utc in a new, undocumented way, and adds a second wrapping layer on core methods that every other plugin (customParseFormat, objectSupport, etc.) also wraps — which is a real risk for plugin-ordering bugs down the line.

The existing tzWall/fixOffset machinery already gives us a host-independent way to resolve "wall clock in zone X" → "real instant" — it's what powers d.tz(string, zone) today. We think set/add(calendar units)/startOf can all be implemented on top of that same primitive: extract the current wall-clock fields via Intl, apply the edit as plain integer arithmetic, resolve back to an instant via fixOffset, and wrap the result — entirely inside src/plugin/timezone/index.js, with $d continuing to represent the real instant everywhere. No changes to Dayjs.prototype or the utc plugin required, and it reuses one already-tested code path instead of introducing a second, parallel one.

So my suggestion: let's merge the original minimal fix now (with the added tests) to close out the reported issue, and open a separate PR for the set/add/startOf DST-correctness work, scoped to stay inside the timezone plugin file. We're happy to help review that follow-up PR once it's up.

@Moumouls

Copy link
Copy Markdown
Contributor Author

@iamkun To keep a full working reference, I suggest we close this PR. I’ll open a new one with a minimal fix, your suggestions, and as many passing tests as possible — without refactor or workarounds. Remaining failing tests will be skipped (but kept into the new PR)

Once that PR is merged, a follow-up PR can do the deeper refactor and make the remaining tests pass, so we still reach the same end state as this PR (#3169).

Reverting the commit here would lose the history and some fixes that could be useful later.

Does that sound good and clear to you?

@iamkun

iamkun commented Aug 13, 2026

Copy link
Copy Markdown
Owner

Sounds good, thanks for the clarification.

One adjustment: for the new minimal-fix PR, let's only include tests that pass. Any failing/edge-case tests (the DST scenarios not yet handled by the minimal fix) should be left out of this PR entirely, not committed-but-skipped — we'd rather every merged PR be fully green. We can track those as follow-up test cases for the deeper-refactor PR.

For that follow-up, please also consider the approach we outlined above: implement the DST-correctness work for set/add/startOf using the existing tzWall/fixOffset primitives, fully contained in src/plugin/timezone/index.js, rather than patching Dayjs.prototype.parse/$set or the utc plugin's valueOf. Happy to discuss once it's up.

@Moumouls

Moumouls commented Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

AI MESSAGE

@iamkun following your review, I opened a minimal PR with only the original tzOffset() change in proto.tz (no wall-clock / set / add / startOf refactor):

#3174

It includes the Guadeloupe regression, the DST-observing target-zone tests you asked for (NY/Paris desync, keepLocalTime, America/New_York and Pacific/Auckland hosts), and the conversion/parse cases from this PR that pass without the later refactor.

This PR (#3169) stays open as the working base for the follow-up (set / add / startOf via tzWall / fixOffset in the timezone plugin only). After #3174 is merged into dev, I will rebase this branch onto dev.

iamkun pushed a commit that referenced this pull request Aug 15, 2026
…3174)

* fix(timezone): compute .tz() offset without host DST

Use tzOffset (Intl formatToParts) in proto.tz instead of
toLocaleString + new Date + getTimezoneOffset, so fixed-offset
zones like America/Guadeloupe keep UTC-4 across host spring DST.

* test(timezone): cover DST-observing zones and host TZ matrix

Add iamkun's requested conversion tests (NY/Paris desync, keepLocalTime,
New_York/Auckland hosts) plus passing parse/convert cases from #3169.
iamkun added a commit that referenced this pull request Aug 16, 2026
* chore: update doc

* chore: update doc

* chore: update doc

* fix(plugin): timezone compute instance .tz() offset without host DST (#3174)

* fix(timezone): compute .tz() offset without host DST

Use tzOffset (Intl formatToParts) in proto.tz instead of
toLocaleString + new Date + getTimezoneOffset, so fixed-offset
zones like America/Guadeloupe keep UTC-4 across host spring DST.

* test(timezone): cover DST-observing zones and host TZ matrix

Add iamkun's requested conversion tests (NY/Paris desync, keepLocalTime,
New_York/Auckland hosts) plus passing parse/convert cases from #3169.

---------

Co-authored-by: Antoine Cormouls <contact.antoine.cormouls@gmail.com>
github-actions Bot pushed a commit that referenced this pull request Aug 16, 2026
## [1.11.22](v1.11.21...v1.11.22) (2026-08-16)

### Bug Fixes

* **plugin:** timezone compute instance .tz() offset without host DST ([#3174](#3174)) ([e27ee80](e27ee80)), closes [#3169](#3169)
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