Remove some internal TimeZone method calls #1397
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1397 +/- ##
===========================================
- Coverage 95.71% 49.03% -46.68%
===========================================
Files 19 18 -1
Lines 11169 4982 -6187
Branches 1812 1089 -723
===========================================
- Hits 10690 2443 -8247
- Misses 476 2128 +1652
- Partials 3 411 +408
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
cjtenny
left a comment
There was a problem hiding this comment.
Looks good. I'll collect observability & ordering of this into the test262 work items issue as well!
43e3e82 to
1ca37be
Compare
|
I went to fix the test262 suite which I had forgotten to do on Friday, and realized that I'd overlooked changing GetOffsetStringFor into BuiltinTimeZoneGetOffsetStringFor. @cjtenny would you mind taking another quick look? |
1ca37be to
a37d835
Compare
| return dateTime; | ||
| getOffsetNanosecondsFor() { | ||
| actual.push("call timeZone.getOffsetNanosecondsFor"); | ||
| return -8735135802468; |
There was a problem hiding this comment.
are these numbers just different keyboard mashings?
There was a problem hiding this comment.
I don't know where 160583136123456789n came from below, but this is the number that yields a PlainDateTime whose time is 12:00:00.987654321.
…ng}For We define the time zone protocol as an object that must have the three methods getOffsetNanosecondsFor, getPossibleInstantsFor, and toString on it. Other methods can be called by user code but should not be called by Temporal. Unfortunately, getInstantFor and getPlainDateTimeFor were called extensively, and getOffsetStringFor in a few places. This probably leaked in to the polyfill and spec text when adding ZonedDateTime! Only the documentation was correct about this.
I noticed this discrepancy between the polyfill and spec text. I believe this is a bug in the spec text, because otherwise if you have a PlainFoo object representing a wall-clock time that doesn't exist or is repeated in the DateTimeFormat's time zone, you could not call toLocaleString() on it.
a37d835 to
d257a01
Compare
Found this bug while investigating #1294.