Partial spec for ZonedDateTime

[ptomato]

This has all the entries for ZonedDateTime, although many of them are marked TODO. Includes a few minor other fixes.

See: #569

[codecov]

Codecov Report

Merging #1044 into main will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1044   +/-   ##
=======================================
  Coverage   94.88%   94.88%           
=======================================
  Files          18       18           
  Lines        7134     7134           
  Branches     1137     1137           
=======================================
  Hits         6769     6769           
  Misses        358      358           
  Partials        7        7           

Flag	Coverage Δ
#test262	45.61% <ø> (ø)
#tests	91.18% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 317c618...030948e. Read the comment docs.

[justingrant]

General question: it's possible that re-building the DateTime for each property access could get quite expensive, esp in cases where you pull multiple props from the same instance which is common. Seems like an implementation may want to cache the DateTime the first time it's generated. Is that something that ECMAScript specs address? Or is that an implementation detail that the implementers worry about but the spec doesn't?

[ptomato]

I planned to find out later on what could be done to address this, so I will definitely follow up later, but my understanding is that it's not possible to cache the DateTime in the case of a custom time zone. For example, you could do this:

class MyTimeZone extends Temporal.TimeZone {
    constructor() { super('bad'); }
    getDateTimeFor() { return Temporal.DateTime.from('1970-01-01'); }
}
const tz = new MyTimeZone();
const zdt = new Temporal.ZonedDateTime(0n, tz);
zdt.year // => 1970
MyTimeZone.prototype.getDateTimeFor = function() { return Temporal.DateTime.from('2000-01-01'); }
zdt.year // => 2000

If we wrote in the spec that the DateTime had to be cached, then you could get into the situation where the result of zdt.year is not equal to zdt.timeZone.getDateTimeFor(new Instant(zdt.epochNanoseconds)).year.

Implementations can cache the calls if the time zone is a built-in one because then the call to getDateTimeFor won't be observable.

[justingrant]

Got it. As long as it's on your radar, that's good enough for me!

[justingrant]

Looks great. Your productivity is astounding! ;-)

[justingrant]

I didn't go back and re-review the ZDT spec after my initial review, and it's a force-push so I'm not sure what changed since your initial checkin and my initial review, so I'm assuming that it's nothing but puppies and rainbows in there. ;-) If there are significant changes in the ZDT spec that you want me to re-review, just LMK. Thanks!

[Ms2ger]

Thanks! Feel free to argue that in favour of deferring any of the comments below to a followup

[Ms2ger]

Will need to be updated for time calendars.

[ptomato]

I'd prefer to leave this and write the spec for time calendars all at once.

[Ms2ger]

How many times do these steps occur now?

[ptomato]

Only twice. They are different for every type (except ZonedDateTime and PlainDateTime, now.)

[ptomato]

(I will follow up on this.)

[Ms2ger]

I think it'll be better to create the DateTime in the constructor and stick it in an internal slot. Followup fine.

[ptomato]

See explanation in #1044 (comment) — the DateTime can't be cached unless the time zone is an unmodified built-in one and Temporal.TimeZone.prototype.getDateTimeFor is %Temporal.TimeZone.prototype.getDateTimeFor%.