Skip to content

Commit

Permalink
Normative: Separate zoned and plain operations in RoundDuration
Browse files Browse the repository at this point in the history
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, only after potentially throwing other errors, and
only once. Previously, it could be converted up to a few separate times in
each operation, such as UnbalanceDurationRelative, RoundDuration, and
BalanceDurationRelative. Since the conversion is user-visible, we don't
want to perform it when not necessary or perform it more times than
necessary.

Closes: #2247
Closes: #2529
  • Loading branch information
ptomato committed Aug 22, 2023
1 parent aaba929 commit 4b7e274
Show file tree
Hide file tree
Showing 4 changed files with 309 additions and 171 deletions.
58 changes: 47 additions & 11 deletions polyfill/lib/duration.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,15 @@ export class Duration {
const maximum = maximumIncrements[smallestUnit];
if (maximum !== undefined) ES.ValidateTemporalRoundingIncrement(roundingIncrement, maximum, false);

let zonedRelativeTo = ES.IsTemporalZonedDateTime(relativeTo) ? relativeTo : undefined;
let plainRelativeTo = ES.IsTemporalDate(relativeTo) ? relativeTo : undefined;

const roundingGranularityIsNoop = smallestUnit === 'nanosecond' && roundingIncrement === 1;
const balancingRequested = largestUnit !== existingLargestUnit;
const calendarUnitsPresent = years !== 0 || months !== 0 || weeks !== 0;
const timeUnitsOverflowWillOccur =
minutes >= 60 || seconds >= 60 || milliseconds >= 1000 || microseconds >= 1000 || nanoseconds >= 1000;
const hoursToDaysConversionMayOccur = (days !== 0 && ES.IsTemporalZonedDateTime(relativeTo)) || hours >= 24;
const hoursToDaysConversionMayOccur = (days !== 0 && zonedRelativeTo) || hours >= 24;
if (
roundingGranularityIsNoop &&
!balancingRequested &&
Expand All @@ -295,13 +298,27 @@ export class Duration {
return new Duration(years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
}

const plainRelativeToWillBeUsed =
smallestUnit === 'year' ||
smallestUnit === 'month' ||
smallestUnit === 'week' ||
years !== 0 ||
months !== 0 ||
weeks !== 0 ||
days !== 0;
if (zonedRelativeTo && plainRelativeToWillBeUsed) {
// Convert a ZonedDateTime relativeTo to PlainDate only if needed in one
// of the operations below, because the conversion is user visible
plainRelativeTo = ES.ToTemporalDate(zonedRelativeTo);
}

({ years, months, weeks, days } = ES.UnbalanceDateDurationRelative(
years,
months,
weeks,
days,
largestUnit,
relativeTo
plainRelativeTo
));
({ years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } =
ES.RoundDuration(
Expand All @@ -318,9 +335,10 @@ export class Duration {
roundingIncrement,
smallestUnit,
roundingMode,
relativeTo
plainRelativeTo,
zonedRelativeTo
));
if (ES.IsTemporalZonedDateTime(relativeTo)) {
if (zonedRelativeTo) {
({ years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } =
ES.AdjustRoundedDurationDays(
years,
Expand All @@ -336,7 +354,7 @@ export class Duration {
roundingIncrement,
smallestUnit,
roundingMode,
relativeTo
zonedRelativeTo
));
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.BalanceTimeDurationRelative(
days,
Expand All @@ -347,7 +365,7 @@ export class Duration {
microseconds,
nanoseconds,
largestUnit,
relativeTo
zonedRelativeTo
));
} else {
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = ES.BalanceTimeDuration(
Expand All @@ -367,7 +385,7 @@ export class Duration {
weeks,
days,
largestUnit,
relativeTo
plainRelativeTo
));

return new Duration(years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds);
Expand Down Expand Up @@ -396,12 +414,29 @@ export class Duration {
const relativeTo = ES.ToRelativeTemporalObject(totalOf);
const unit = ES.GetTemporalUnit(totalOf, 'unit', 'datetime', ES.REQUIRED);

let zonedRelativeTo = ES.IsTemporalZonedDateTime(relativeTo) ? relativeTo : undefined;
let plainRelativeTo = ES.IsTemporalDate(relativeTo) ? relativeTo : undefined;
const plainRelativeToWillBeUsed =
unit === 'year' || unit === 'month' || unit === 'week' || years !== 0 || months !== 0 || weeks !== 0;
if (zonedRelativeTo !== undefined && plainRelativeToWillBeUsed) {
// Convert a ZonedDateTime relativeTo to PlainDate only if needed in one
// of the operations below, because the conversion is user visible
plainRelativeTo = ES.ToTemporalDate(zonedRelativeTo);
}

// Convert larger units down to days
({ years, months, weeks, days } = ES.UnbalanceDateDurationRelative(years, months, weeks, days, unit, relativeTo));
({ years, months, weeks, days } = ES.UnbalanceDateDurationRelative(
years,
months,
weeks,
days,
unit,
plainRelativeTo
));
// If the unit we're totalling is smaller than `days`, convert days down to that unit.
let balanceResult;
if (ES.IsTemporalZonedDateTime(relativeTo)) {
const intermediate = ES.MoveRelativeZonedDateTime(relativeTo, years, months, weeks, 0);
if (zonedRelativeTo) {
const intermediate = ES.MoveRelativeZonedDateTime(zonedRelativeTo, years, months, weeks, 0);
balanceResult = ES.BalancePossiblyInfiniteTimeDurationRelative(
days,
hours,
Expand Down Expand Up @@ -446,7 +481,8 @@ export class Duration {
1,
unit,
'trunc',
relativeTo
plainRelativeTo,
zonedRelativeTo
);
return total;
}
Expand Down
Loading

0 comments on commit 4b7e274

Please sign in to comment.