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

RoundDuration: Consider calling ToTemporalDate(relativeTo) only when needed #2247

Closed
anba opened this issue May 31, 2022 · 1 comment · Fixed by #2657
Closed

RoundDuration: Consider calling ToTemporalDate(relativeTo) only when needed #2247

anba opened this issue May 31, 2022 · 1 comment · Fixed by #2657
Labels
non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! normative Would be a normative change to the proposal spec-text Specification text involved
Milestone

Comments

@anba
Copy link
Contributor

anba commented May 31, 2022

RoundDuration, step 6 is only executed when needed:

  1. If unit is one of "year", "month", "week", or "day", then
    ...

But step 4 is unconditionally executed, even though relativeTo is only needed for "year", "month", and "week":

  1. If relativeTo is not undefined, then
    ...

This would be an observable change, because ToTemporalDate(relativeTo) can call user code.

@ptomato ptomato added spec-text Specification text involved non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! normative Would be a normative change to the proposal labels Jul 4, 2022
ptomato added a commit that referenced this issue Jul 6, 2022
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

Closes: #2247
@ptomato
Copy link
Collaborator

ptomato commented Jul 6, 2022

There are several other places where a ZonedDateTime relativeTo is converted with ToTemporalDate during a duration.round() operation, including in UnbalanceDurationRelative. We should make sure it is converted only once.

@ptomato ptomato added this to the Stage "3.5" milestone Dec 8, 2022
ptomato added a commit that referenced this issue Mar 11, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Mar 11, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Mar 11, 2023
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, 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
ptomato added a commit that referenced this issue Apr 22, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Apr 22, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Apr 22, 2023
This converts a ZonedDateTime relativeTo into a PlainDateTime relativeTo
only when necessary, 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
ptomato added a commit that referenced this issue Apr 26, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Apr 26, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Apr 26, 2023
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
ptomato added a commit that referenced this issue Apr 27, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Apr 27, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Apr 27, 2023
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
ptomato added a commit that referenced this issue May 3, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue May 3, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue May 3, 2023
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
ptomato added a commit that referenced this issue May 11, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue May 11, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue May 11, 2023
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
ptomato added a commit that referenced this issue May 13, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue May 13, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue May 13, 2023
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
ptomato added a commit that referenced this issue May 13, 2023
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
ptomato added a commit that referenced this issue May 19, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Jun 12, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Jun 12, 2023
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
ptomato added a commit that referenced this issue Jun 20, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Jun 20, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Jun 20, 2023
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
ptomato added a commit that referenced this issue Aug 18, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Aug 18, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Aug 18, 2023
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
ptomato added a commit that referenced this issue Aug 18, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Aug 18, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Aug 18, 2023
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
ptomato added a commit that referenced this issue Aug 22, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Aug 22, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Aug 22, 2023
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
ptomato added a commit that referenced this issue Aug 26, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Aug 26, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Aug 26, 2023
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
ptomato added a commit that referenced this issue Sep 11, 2023
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
ptomato added a commit that referenced this issue Sep 12, 2023
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
ptomato added a commit that referenced this issue Sep 12, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Sep 12, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Sep 12, 2023
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
ptomato added a commit that referenced this issue Sep 13, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Sep 13, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Sep 13, 2023
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
ptomato added a commit that referenced this issue Sep 13, 2023
…uration

relativeTo as a PlainDate is only needed when smallestUnit is year, month,
or week. relativeTo as a ZonedDateTime is used additionally when
smallestUnit is day. However, ToTemporalDate only needs to be called in
the former case. Since ToTemporalDate potentially calls user code,
rearrange some steps to make sure to call it only when necessary.

See: #2247
ptomato added a commit that referenced this issue Sep 13, 2023
…mpare

We call UnbalanceDurationRelative twice with the same relativeTo object.
UnbalanceDurationRelative only uses plain or undefined relativeTo, not
zoned, so it will convert it with ToTemporalDate. No need to do this
twice; pre-convert it before the first call.

See: #2247
ptomato added a commit that referenced this issue Sep 13, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! normative Would be a normative change to the proposal spec-text Specification text involved
Projects
None yet
2 participants