Normative: Validation of NanosecondsToDays and getOffsetNanoseconds #2387
Conversation
|
I'm not sure if we want all of these, especially the third commit. cc @anba |
Codecov Report
@@ Coverage Diff @@
## main #2387 +/- ##
==========================================
- Coverage 94.70% 94.65% -0.05%
==========================================
Files 20 20
Lines 11053 11062 +9
Branches 1965 1969 +4
==========================================
+ Hits 10468 10471 +3
- Misses 540 544 +4
- Partials 45 47 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
I don't think we should take the third commit. Are we sure that all callers of GetOffsetNanosecondsFor work correctly when the offset nanoseconds can be any value, including values like |
922add3 to
37df41b
Compare
|
I've removed the third commit for now, and plan to present this at the September TC39 plenary. We can decide later about the advantages and disadvantages of removing the limit on the return value of getOffsetNanosecondsFor. |
37df41b to
8996c4d
Compare
|
You're right, those were left over from a different way that I originally tried to do this. |
When converting a number of nanoseconds to a number of days and a nanoseconds remainder, neither the resulting number of days nor the nanoseconds remainder should have the opposite sign as the original number of nanoseconds. This could happen due to shenanigans in a custom time zone's getOffsetNanosecondsFor or getPossibleInstantsFor methods, or a custom calendar's dateAdd method. See: #2357
When converting a number of nanoseconds to a number of days and a nanoseconds remainder, the remainder shouldn't be longer than the length of the last day. This could happen due to shenanigans in a custom time zone's getOffsetNanosecondsFor or getPossibleInstantsFor methods, or a custom calendar's dateAdd method. See: #2357
Adds code to the NYSE time zone example in the cookbook which exercises the interaction between ZonedDateTime arithmetic and time zone UTC offset changes.
8996c4d to
1f500a5
Compare
|
I'm probably not going to get a chance to write test262 tests for this for another couple of weeks, as I'll be occupied with other things. So I've added it to the general "test262 work items" list (#1398). I think we should go ahead and review and merge this now, as it's the last of the normative changes that were already approved in July and it's already been quite a while that it hasn't been reflected in the official proposal text. |
Removal of 24-hour limit for getOffsetNanoseconds - this limit was leading to inconsistencies in custom time zones (see cookbook example)Closes: #2357