Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normative: Fix date difference for end-of-month edge cases #2759

Merged
merged 2 commits into from
Feb 10, 2024

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Jan 27, 2024

This adjusts the difference algorithm for Gregorian-year dates so that when an intermediate date occurs past the end of a month, it is not shifted to the end of that month.

Previously, in some edge cases where taking the difference in months or years would return a number of months and zero days, we now return one month less and 28, 29, or 30 days instead.

Example: 1970-01-29 until 1971-02-28, largestUnit years
Old result: 1 year, 1 month
New result: 1 year, 30 days

Note that largestUnit weeks and largestUnit days, the latter of which is the default, are not affected.

(Note: Stacked on top of an editorial change which I will eventually make a separate PR for.)

Closes: #2535

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (7bd3589) 96.59% compared to head (74f3dbb) 96.57%.
Report is 2 commits behind head on main.

Files Patch % Lines
polyfill/lib/ecmascript.mjs 95.45% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2759      +/-   ##
==========================================
- Coverage   96.59%   96.57%   -0.02%     
==========================================
  Files          23       23              
  Lines       12218    12242      +24     
  Branches     2263     2268       +5     
==========================================
+ Hits        11802    11823      +21     
- Misses        356      358       +2     
- Partials       60       61       +1     

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

@ptomato ptomato marked this pull request as draft January 27, 2024 02:12
This adjusts the difference algorithm for Gregorian-year dates so that
when an intermediate date occurs past the end of a month, it is not
shifted to the end of that month.

Previously, in some edge cases where taking the difference in months or
years would return a number of months and zero days, we now return one
month less and 28, 29, or 30 days instead.

Example: 1970-01-29 until 1971-02-28, largestUnit years
Old result: 1 year, 1 month
New result: 1 year, 30 days

Note that largestUnit weeks and largestUnit days, the latter of which is
the default, are not affected.

Closes: #2535
ptomato added a commit to ptomato/test262 that referenced this pull request Feb 7, 2024
This adds tests specifically for every kind of case that changes due to
the tweak to the date difference algorithm: differences from a longer
month to a shorter month, when the months are adjacent, in the same year
but not adjacent, and in different years.

Also adds tests for a case that does *not* change, but would trip on an
incorrectly implemented algorithm: when the intermediate months value
falls at the end of February.

There was incidental coverage of the change to the date difference
algorithm in other tests. Those are adjusted, as well.

Normative change: tc39/proposal-temporal#2759
Consensus in February 2024
ptomato added a commit to ptomato/test262 that referenced this pull request Feb 9, 2024
This adds tests specifically for every kind of case that changes due to
the tweak to the date difference algorithm: differences from a longer
month to a shorter month, when the months are adjacent, in the same year
but not adjacent, and in different years.

Also adds tests for a case that does *not* change, but would trip on an
incorrectly implemented algorithm: when the intermediate months value
falls at the end of February.

There was incidental coverage of the change to the date difference
algorithm in other tests. Those are adjusted, as well.

Normative change: tc39/proposal-temporal#2759
Consensus in February 2024
ptomato added a commit to tc39/test262 that referenced this pull request Feb 10, 2024
This adds tests specifically for every kind of case that changes due to
the tweak to the date difference algorithm: differences from a longer
month to a shorter month, when the months are adjacent, in the same year
but not adjacent, and in different years.

Also adds tests for a case that does *not* change, but would trip on an
incorrectly implemented algorithm: when the intermediate months value
falls at the end of February.

There was incidental coverage of the change to the date difference
algorithm in other tests. Those are adjusted, as well.

Normative change: tc39/proposal-temporal#2759
Consensus in February 2024
@ptomato ptomato marked this pull request as ready for review February 10, 2024 00:04
@ptomato
Copy link
Collaborator Author

ptomato commented Feb 10, 2024

This change gained consensus at TC39 on 2024-02-06.

@ptomato ptomato force-pushed the 2535-end-of-month-edge-cases branch from 4350921 to 74f3dbb Compare February 10, 2024 00:13
@ptomato ptomato merged commit 847a8bb into main Feb 10, 2024
9 checks passed
@ptomato ptomato deleted the 2535-end-of-month-edge-cases branch February 10, 2024 00:18
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 3, 2024
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request Dec 6, 2024
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.

Inconsistent results in DifferenceISODate?
2 participants