tc39 / proposal-intl-duration-format

https://tc39.es/proposal-intl-duration-format
MIT License
165 stars 18 forks source link

Normative: Limit valid values for DurationFormats to match upcoming limits in Temporal #173

Closed ben-allen closed 4 months ago

ben-allen commented 11 months ago

See #157

Upcoming revisions to Temporal will limit the valid values for Temporal.Duration. This commit applies the same limits on valid values for Intl.DurationFormat DurationRecords. Specifically: the absolute values of the values stored in each of the [[Years]], [[Months]], and [[Weeks]] fields cannot exceed 2^32, and the absolute value of the value obtained through converting the values in each of the sub-week fields to seconds and then summing them must not exceed 2^53.

sffc commented 11 months ago

TG2 approval: https://github.com/tc39/ecma402/blob/master/meetings/notes-2023-11-16.md#decide-if-upcoming-temporalduration-limits-should-have-an-impact-on-durationformat-157

ryzokuken commented 10 months ago

2023-11-28: This PR achieved TC39-TG1 consensus.

anba commented 9 months ago

Test coverage → https://github.com/tc39/test262/pull/3988

ben-allen commented 5 months ago

Updated to match limits currently set in Temporal. See: https://github.com/tc39/proposal-temporal/pull/2727#

@ptomato has pointed me toward https://github.com/tc39/proposal-temporal/pull/2727#discussion_r1408573795 for thoughts on implementation. He's also written a polyfill that implements the limits via string manipulation to run test262 tests against.

@ryzokuken @FrankYFTang

ptomato commented 5 months ago

Note the text in Temporal PR 2727 was not the latest, Anba made some changes to it afterwards.

ben-allen commented 5 months ago

Note the text in Temporal PR 2727 was not the latest, Anba made some changes to it afterwards.

Just pushed a corrected version. Thanks for catching that!

syg commented 5 months ago

Does this change mean BigInt math is not required (just like in Temporal)?

FrankYFTang commented 5 months ago

Could we do the following change to better match Temporal?

  1. in ToDurationRecord change
  2. If IsValidDurationRecord(result) is false, throw a RangeError exception.
    to
  3. If IsValidDuration( result.[[Years]] , result.[[Months]] , result.[[Weeks]] , result.[[Days]] , result.[[Hours]] , result.[[Minutes]] , result.[[Seconds]] , result.[[Milliseconds]] , result.[[Microseconds]] , result.[[Nanoseconds]] ) is false, throw a RangeError exception.

and change this part to be the exact text of

7.5.15 IsValidDuration ( years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds ) from Temporal spec (probabaly except replacing the "to construct a Temporal.Duration" part to "to construct a DurationRecord"

FrankYFTang commented 4 months ago

Notice, with the PR , some test cases in https://github.com/tc39/test262/blob/main/test/intl402/DurationFormat/prototype/format/precision-exact-mathematical-values.js

will be invalid and require changes to test the boundary condition and throwing/not throwing in the correct values