Skip to content

Commit

Permalink
Normative: Combine code paths for duration rounding and difference
Browse files Browse the repository at this point in the history
In order to prevent bugs due to discrepancies between two ways of
calculating the same thing such as in #2742, refactor duration rounding
with relativeTo so that

    duration.round({ smallestUnit, largestUnit, relativeTo, ...options })

goes through the same code path and gives the same result as

    const target = relativeTo.add(duration);
    relativeTo.until(target, { smallestUnit, largestUnit, ...options })

but taking into account that the until() methods have a different default
roundingMode than Duration.prototype.round(), and optimizing away as many
user-observable calls as possible.

Similarly,

    duration.total({ unit, relativeTo, ...options })

goes through the same code path, which also returns the total as a
mathematical value if needed.
  • Loading branch information
ptomato committed Feb 3, 2024
1 parent 36d77d6 commit b3985a4
Show file tree
Hide file tree
Showing 6 changed files with 566 additions and 384 deletions.
262 changes: 140 additions & 122 deletions polyfill/lib/duration.mjs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* global __debug__ */

import * as ES from './ecmascript.mjs';
import { MakeIntrinsicClass } from './intrinsicclass.mjs';
import { GetIntrinsic, MakeIntrinsicClass } from './intrinsicclass.mjs';
import { CalendarMethodRecord } from './methodrecord.mjs';
import {
YEARS,
Expand All @@ -17,6 +17,9 @@ import {
CALENDAR,
INSTANT,
EPOCHNANOSECONDS,
ISO_YEAR,
ISO_MONTH,
ISO_DAY,
CreateSlots,
GetSlot,
SetSlot
Expand Down Expand Up @@ -305,11 +308,7 @@ export class Duration {

let precalculatedPlainDateTime;
const plainDateTimeOrRelativeToWillBeUsed =
!roundingGranularityIsNoop ||
ES.IsCalendarUnit(largestUnit) ||
largestUnit === 'day' ||
calendarUnitsPresent ||
days !== 0;
ES.IsCalendarUnit(largestUnit) || largestUnit === 'day' || calendarUnitsPresent || days !== 0;
if (zonedRelativeTo && plainDateTimeOrRelativeToWillBeUsed) {
// Convert a ZonedDateTime relativeTo to PlainDateTime and PlainDate only
// if either is needed in one of the operations below, because the
Expand All @@ -327,79 +326,85 @@ export class Duration {
'dateUntil'
]);

({ years, months, weeks, days } = ES.UnbalanceDateDurationRelative(
years,
months,
weeks,
days,
largestUnit,
plainRelativeTo,
calendarRec
));
let norm = TimeDuration.normalize(hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
({ years, months, weeks, days, norm } = ES.RoundDuration(
years,
months,
weeks,
days,
norm,
roundingIncrement,
smallestUnit,
roundingMode,
plainRelativeTo,
calendarRec,
zonedRelativeTo,
timeZoneRec,
precalculatedPlainDateTime
));

if (zonedRelativeTo) {
({ years, months, weeks, days, norm } = ES.AdjustRoundedDurationDays(
years,
months,
weeks,
days,
norm,
roundingIncrement,
smallestUnit,
roundingMode,
zonedRelativeTo,
calendarRec,
const relativeEpochNs = GetSlot(zonedRelativeTo, EPOCHNANOSECONDS);
const targetEpochNs = ES.AddZonedDateTime(
GetSlot(zonedRelativeTo, INSTANT),
timeZoneRec,
precalculatedPlainDateTime
));
const intermediate = ES.MoveRelativeZonedDateTime(
zonedRelativeTo,
calendarRec,
timeZoneRec,
years,
months,
weeks,
0,
precalculatedPlainDateTime
);
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.BalanceTimeDurationRelative(
days,
norm,
largestUnit,
intermediate,
timeZoneRec
));
precalculatedPlainDateTime
);
({ years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } =
ES.DifferenceZonedDateTimeWithRounding(
relativeEpochNs,
targetEpochNs,
plainRelativeTo,
calendarRec,
zonedRelativeTo,
timeZoneRec,
precalculatedPlainDateTime,
ObjectCreate(null),
largestUnit,
roundingIncrement,
smallestUnit,
roundingMode
));
} else if (plainRelativeTo) {
let targetTime = ES.AddTime(0, 0, 0, 0, 0, 0, norm);

// Delegate the date part addition to the calendar
const TemporalDuration = GetIntrinsic('%Temporal.Duration%');
const dateDuration = new TemporalDuration(years, months, weeks, days + targetTime.deltaDays, 0, 0, 0, 0, 0, 0);
const targetDate = ES.AddDate(calendarRec, plainRelativeTo, dateDuration);

({ years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } =
ES.DifferencePlainDateTimeWithRounding(
plainRelativeTo,
0,
0,
0,
0,
0,
0,
GetSlot(targetDate, ISO_YEAR),
GetSlot(targetDate, ISO_MONTH),
GetSlot(targetDate, ISO_DAY),
targetTime.hour,
targetTime.minute,
targetTime.second,
targetTime.millisecond,
targetTime.microsecond,
targetTime.nanosecond,
calendarRec,
largestUnit,
roundingIncrement,
smallestUnit,
roundingMode
));
} else {
// No reference date to calculate difference relative to
if (calendarUnitsPresent) {
throw new RangeError('a starting point is required for years, months, or weeks balancing');
}
if (ES.IsCalendarUnit(largestUnit)) {
throw new RangeError(`a starting point is required for ${largestUnit}s balancing`);
}
if (ES.IsCalendarUnit(smallestUnit)) {
throw new RangeError(`a starting point is required for ${smallestUnit}s rounding`);
}
({ days, norm } = ES.RoundDuration(0, 0, 0, days, norm, roundingIncrement, smallestUnit, roundingMode));
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.BalanceTimeDuration(
norm.add24HourDays(days),
largestUnit
));
}
({ years, months, weeks, days } = ES.BalanceDateDurationRelative(
years,
months,
weeks,
days,
largestUnit,
smallestUnit,
plainRelativeTo,
calendarRec
));

return new Duration(years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
}
Expand Down Expand Up @@ -446,69 +451,80 @@ export class Duration {
'dateUntil'
]);

// Convert larger units down to days
({ years, months, weeks, days } = ES.UnbalanceDateDurationRelative(
years,
months,
weeks,
days,
unit,
plainRelativeTo,
calendarRec
));
let norm;
// If the unit we're totalling is smaller than `days`, convert days down to that unit.
let norm = TimeDuration.normalize(hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
if (zonedRelativeTo) {
const intermediate = ES.MoveRelativeZonedDateTime(
zonedRelativeTo,
calendarRec,
const relativeEpochNs = GetSlot(zonedRelativeTo, EPOCHNANOSECONDS);
const targetEpochNs = ES.AddZonedDateTime(
GetSlot(zonedRelativeTo, INSTANT),
timeZoneRec,
calendarRec,
years,
months,
weeks,
0,
days,
norm,
precalculatedPlainDateTime
);
norm = TimeDuration.normalize(hours, minutes, seconds, milliseconds, microseconds, nanoseconds);

// Inline BalanceTimeDurationRelative, without the final balance step
const start = GetSlot(intermediate, INSTANT);
const startNs = GetSlot(intermediate, EPOCHNANOSECONDS);
let intermediateNs = startNs;
let startDt;
if (days !== 0) {
startDt = ES.GetPlainDateTimeFor(timeZoneRec, start, 'iso8601');
intermediateNs = ES.AddDaysToZonedDateTime(start, startDt, timeZoneRec, 'iso8601', days).epochNs;
}
const endNs = ES.AddInstant(intermediateNs, norm);
norm = TimeDuration.fromEpochNsDiff(endNs, startNs);
if (ES.IsCalendarUnit(unit) || unit === 'day') {
if (!norm.isZero()) startDt ??= ES.GetPlainDateTimeFor(timeZoneRec, start, 'iso8601');
({ days, norm } = ES.NormalizedTimeDurationToDays(norm, intermediate, timeZoneRec, startDt));
} else {
days = 0;
}
} else {
norm = TimeDuration.normalize(hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
norm = norm.add24HourDays(days);
days = 0;
const { total } = ES.DifferenceZonedDateTimeWithRounding(
relativeEpochNs,
targetEpochNs,
plainRelativeTo,
calendarRec,
zonedRelativeTo,
timeZoneRec,
precalculatedPlainDateTime,
ObjectCreate(null),
unit,
1,
unit,
'trunc'
);
return total;
}
// Finally, truncate to the correct unit and calculate remainder
const { total } = ES.RoundDuration(
years,
months,
weeks,
days,
norm,
1,
unit,
'trunc',
plainRelativeTo,
calendarRec,
zonedRelativeTo,
timeZoneRec,
precalculatedPlainDateTime
);

if (plainRelativeTo) {
let targetTime = ES.AddTime(0, 0, 0, 0, 0, 0, norm);

// Delegate the date part addition to the calendar
const TemporalDuration = GetIntrinsic('%Temporal.Duration%');
const dateDuration = new TemporalDuration(years, months, weeks, days + targetTime.deltaDays, 0, 0, 0, 0, 0, 0);
const targetDate = ES.AddDate(calendarRec, plainRelativeTo, dateDuration);

const { total } = ES.DifferencePlainDateTimeWithRounding(
plainRelativeTo,
0,
0,
0,
0,
0,
0,
GetSlot(targetDate, ISO_YEAR),
GetSlot(targetDate, ISO_MONTH),
GetSlot(targetDate, ISO_DAY),
targetTime.hour,
targetTime.minute,
targetTime.second,
targetTime.millisecond,
targetTime.microsecond,
targetTime.nanosecond,
calendarRec,
unit,
1,
unit,
'trunc'
);
return total;
}

// No reference date to calculate difference relative to
if (years !== 0 || months !== 0 || weeks !== 0) {
throw new RangeError('a starting point is required for years, months, or weeks total');
}
if (ES.IsCalendarUnit(unit)) {
throw new RangeError(`a starting point is required for ${unit}s total`);
}
norm = norm.add24HourDays(days);
const { total } = ES.RoundDuration(0, 0, 0, 0, norm, 1, unit, 'trunc');
return total;
}
toString(options = undefined) {
Expand Down Expand Up @@ -707,9 +723,11 @@ export class Duration {
}

if (calendarUnitsPresent) {
// plainRelativeTo may be undefined, and if so Unbalance will throw
({ days: d1 } = ES.UnbalanceDateDurationRelative(y1, mon1, w1, d1, 'day', plainRelativeTo, calendarRec));
({ days: d2 } = ES.UnbalanceDateDurationRelative(y2, mon2, w2, d2, 'day', plainRelativeTo, calendarRec));
if (!plainRelativeTo) {
throw new RangeError('A starting point is required for years, months, or weeks comparison');
}
d1 = ES.UnbalanceDateDurationRelative(y1, mon1, w1, d1, plainRelativeTo, calendarRec);
d2 = ES.UnbalanceDateDurationRelative(y2, mon2, w2, d2, plainRelativeTo, calendarRec);
}
const norm1 = TimeDuration.normalize(h1, min1, s1, ms1, µs1, ns1).add24HourDays(d1);
const norm2 = TimeDuration.normalize(h2, min2, s2, ms2, µs2, ns2).add24HourDays(d2);
Expand Down
Loading

0 comments on commit b3985a4

Please sign in to comment.