Skip to content

Handle double midnight in ZonedDateTime.round#3315

Merged
ptomato merged 4 commits into
mainfrom
3312-double-midnight
May 21, 2026
Merged

Handle double midnight in ZonedDateTime.round#3315
ptomato merged 4 commits into
mainfrom
3312-double-midnight

Conversation

@ptomato
Copy link
Copy Markdown
Collaborator

@ptomato ptomato commented May 8, 2026

Tentative solution to #3312. If we like this solution, I'll make the change in the spec text.

@ptomato ptomato added the run snapshot tests Add this label to a PR if you want the snapshot tests to run on it. label May 8, 2026
@ptomato ptomato marked this pull request as draft May 8, 2026 01:29
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.39%. Comparing base (7083e8c) to head (81a6886).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3315   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files          22       22           
  Lines       10763    10796   +33     
  Branches     1866     1875    +9     
=======================================
+ Hits        10590    10623   +33     
  Misses        161      161           
  Partials       12       12           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Collaborator

@justingrant justingrant left a comment

Choose a reason for hiding this comment

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

I think this is fine? Not 100% confident but it seems reasonable to handle the one known case of this.

Comment thread polyfill/lib/zoneddatetime.mjs Outdated
@justingrant
Copy link
Copy Markdown
Collaborator

Although the snapshot tests fail. Is that expected?

@ptomato
Copy link
Copy Markdown
Collaborator Author

ptomato commented May 12, 2026

Although the snapshot tests fail. Is that expected?

Well, I did forget to update the snapshot files, which I've done now. But one of them still fails because the Antarctica/Casey case also hits the assertion in #3311, so we won't get a clean run until we have a solution for that.

@ptomato
Copy link
Copy Markdown
Collaborator Author

ptomato commented May 12, 2026

Proposing the spec change in https://ptomato.name/talks/tc39-2026-05/#6.

@ptomato ptomato marked this pull request as ready for review May 19, 2026 15:05
@ptomato ptomato force-pushed the 3312-double-midnight branch from 1f65a30 to f7805ae Compare May 19, 2026 15:34
@ptomato
Copy link
Copy Markdown
Collaborator Author

ptomato commented May 19, 2026

The needs-consensus PR that corresponds to this fix reached TC39 consensus in the meeting of 2026-05-19.

@ptomato
Copy link
Copy Markdown
Collaborator Author

ptomato commented May 19, 2026

I removed the new case from the snapshot files, since it doesn't fully pass them until we also fix #3311. I'll re-add it then.

ptomato added 2 commits May 21, 2026 10:51
For all of our interesting ZonedDateTimes, test the startOfDay() and
hoursInDay invariants.

See discussion in #3312
The assertion in the previous code was a problem in cases where a
backwards UTC shift spanned across midnight (i.e. from a time after
midnight to a time before midnight, not starting or ending at midnight.)

It would produce a span of startNs...thisNs that was longer than 100% of
the day length, which failed the assertion, but would also mess up
rounding modes (would expand to 200% of the day length, for example.)

See: #3312
@ptomato ptomato force-pushed the 3312-double-midnight branch from f7805ae to 564458f Compare May 21, 2026 17:55
ptomato added 2 commits May 21, 2026 11:27
Recently new test262 tests were added that test the order of observable
reads in the DateTimeFormat constructor more strictly. Property reads
should be interleaved with the observable calls to toString/valueOf.
Implement this in our polyfilled DateTimeFormat constructor so that it
adheres to the spec and passes the test262 tests.
@ptomato ptomato force-pushed the 3312-double-midnight branch from 564458f to 81a6886 Compare May 21, 2026 18:27
@ptomato ptomato merged commit 2c71cc9 into main May 21, 2026
10 checks passed
@ptomato ptomato deleted the 3312-double-midnight branch May 21, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run snapshot tests Add this label to a PR if you want the snapshot tests to run on it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants