Skip to content

Fix calendar-unit rounding expanding an exact duration#364

Closed
spokodev wants to merge 1 commit into
js-temporal:mainfrom
spokodev:fix/nudge-calendar-startbound
Closed

Fix calendar-unit rounding expanding an exact duration#364
spokodev wants to merge 1 commit into
js-temporal:mainfrom
spokodev:fix/nudge-calendar-startbound

Conversation

@spokodev

@spokodev spokodev commented Jul 2, 2026

Copy link
Copy Markdown

Calendar-unit rounding with a directional rounding mode (ceil, floor, expand) expands a duration that already lands exactly on the smallest-unit boundary, giving an off-by-one-unit result.

Temporal.PlainDate.from('2020-01-15').until('2021-01-15', {
  largestUnit: 'year', smallestUnit: 'month', roundingMode: 'ceil'
}).toString(); // 'P1Y1M', should be 'P1Y' (the dates are exactly 12 months apart)

Temporal.Duration.from({ years: 1 }).round({
  largestUnit: 'year', smallestUnit: 'month', roundingMode: 'ceil', relativeTo
}).toString(); // 'P1Y1M', should be 'P1Y'

Reachable from Duration.round and {PlainDate,PlainDateTime,ZonedDateTime}.{until,since}. Duration.total is unaffected.

Cause

In NudgeToCalendarUnit, the start bound of the rounding window is chosen with if (!r1), where r1 is only the rounded smallest-unit field. When r1 is zero but the rest of startDuration is non-zero (for example rounding P1Y to months, where startDuration keeps years: 1), it treats the origin instant (relativeTo) as the start bound, even though startDuration actually ends one year later. That makes the bounding numerator non-zero, so a directional rounding mode rounds up to r2.

Fix

Guard on DateDurationSign(startDuration) === 0 instead of !r1, so the origin shortcut is used only when the whole start duration is zero. This matches the current reference algorithm (ComputeNudgeWindow in tc39/proposal-temporal). DateDurationSign already exists in the same file.

Testing

Added test/round-nudge-startbound.mjs covering the ceil, floor, and Duration.round cases above. They return P1Y1M/-P1Y1M on the current code and the correct P1Y/-P1Y with the fix. The full test suite stays green.

NudgeToCalendarUnit chose the start bound of the rounding window with
'if (!r1)', looking only at the rounded smallest-unit field. When that
field is zero but the rest of startDuration is not (e.g. rounding P1Y to
months), it used the origin instant as the start bound instead of the
real endpoint of startDuration, so the numerator was non-zero and a
directional rounding mode expanded an already-exact result (P1Y became
P1Y1M for until/since/round with ceil or floor). Guard on
DateDurationSign(startDuration) === 0, matching the reference algorithm.
@ptomato

ptomato commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Thanks for taking the time to contribute! This fix and its corresponding tests will already be pulled in on our next rebase from https://github.com/tc39/proposal-temporal, so I'll close this. Just watch for #361 and its follow-ups to be merged.

@ptomato ptomato closed this Jul 2, 2026
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.

2 participants