inconsistency in rounding days with a large increment

[BurntSushi]

I have this code snippet:

import { Temporal } from 'temporal-polyfill';

let d1 = Temporal.Duration.from({days: 65});
let dt1 = Temporal.PlainDateTime.from('2024-06-19 00:00:00');
let d2 = d1.round({
  relativeTo: dt1,
  largestUnit: 'year',
  smallestUnit: 'day',
  roundingIncrement: 35,
});
console.log(d2.toString());

Using this polyfill, the output is 2m. But with @js-temporal/polyfill, I'm getting 2m9d. I think the latter is correct. From what I can tell, the issue is here:

https://github.com/fullcalendar/temporal-polyfill/blob/2d3fd89f21eac311eceee9151640072911fb6467/packages/temporal-polyfill/src/internal/round.ts#L538-L544

Namely, this is rounding the duration based only on the time/day component. But it is neglecting the days contributed by units higher than days. Which in this case is 2 months since the duration, at this point, has been balanced.

(I am working on a datetime library for Rust inspired by Temporal, and I found this case by trying weird values of roundingIncrement.)

[BurntSushi]

I think the the same inconsistency exists in zone aware rounding too, e.g., 2024-06-19 00:00:00[America/New_York], even though that (I believe) goes through a different code path.

[arshaw]

Hi @BurntSushi, thanks for using this package and for exploring this edge case. The temporal spec changed a little while ago, but the @js-temporal/polyfill package did NOT yet update to it. However, temporal-polyfill DID. So temporal-polyfill implements the newer spec for diffing/rounding. If you go to https://tc39.es/proposal-temporal/docs/ and open up the console to use the "playground" you'll see the answer is 'P2M'.

The rounding logic in the spec was refactored for a number of reasons (I actually helped out with it) but one of the outcomes is that roundingIncrement only operates "within" the parent unit. So, if the unrounded result is P2M4D, then the only the 4D is being rounded, within 0-35, and of course it rounds down to 0.

But I'm glad you brought this up because rounding UP could yield strange results. I've opened a TC39 ticket here: https://github.com/tc39/proposal-temporal/issues/2902

[BurntSushi]

Oh nice! That's a relief. It is definitely an odd case and I wasn't fully sure what the right answer ought to be, but what y'all settled on seems sensible. And thanks for the tip on using the console on the doc web site.

The rounding logic in the spec was refactored for a number of reasons (I actually helped out with it)

Yeah I've been following this and really appreciated the gist you provided. It was quite timely too, because I was dreading implementing duration rounding before then. But your approach massively simplified it.

[arshaw]

No problem at all.

Yeah I've been following this and really appreciated the gist you provided

That makes me very happy, I'm glad others got some good use out of that!