Time zone and calendar protocol best practices #1043
Conversation
ptomato
requested review from
Ms2ger,
justingrant,
pipobscure,
jasonwilliams,
sffc,
gibson042,
mattjohnsonpint and
ryzokuken
Codecov Report
@@ Coverage Diff @@
## main #1043 +/- ##
==========================================
- Coverage 94.30% 93.99% -0.32%
==========================================
Files 18 18
Lines 6657 6662 +5
Branches 998 1048 +50
==========================================
- Hits 6278 6262 -16
- Misses 372 391 +19
- Partials 7 9 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
docs/calendar.md
Outdated
| It must not have a `calendar` property, so that it can be distinguished in `Temporal.Calendar.from()` from other Temporal objects that have a calendar. | ||
| It is possible to pass such an object into any Temporal API that would normally take a built-in `Temporal.Calendar`. | ||
|
|
||
| Any implementation of the protocol with the required methods will return the correct output from any Temporal property or method. | ||
| However, most other code will assume that custom calendars act like built-in `Temporal.Calendar` objects. | ||
| If you want to interoperate with libraries or other code that you didn't write, then you should implement the `id` property and the `fields()` method as well. |
There was a problem hiding this comment.
I don't get why id and fields() are optional. What happens if Temporal gets a calendar without those fields and some method that needs them is called (like toString for .id or getFields() for fields()?)
There was a problem hiding this comment.
Note that id defers to toString, not the other way around.
There was a problem hiding this comment.
If fields isn't present on the object, then %Temporal.Calendar.prototype.fields% is called, which just clones the input array. It's optional to override fields if you subclass, so it should be optional to provide it on a plain-object calendar. This is consistent with what we do for the TimeZone methods that are optional to override when you subclass TimeZone.
The default implementation of TimeZone.getDateTimeFor() uses TimeZone.getOffsetNanosecondsFor(), so it's not required to implement this method in the protocol.
We have TimeZone.id optional on plain objects, in favour of requiring TimeZone.toString(). The calendar protocol should do the same thing.
Emphasize that inheriting from the built-in objects is the recommended way to create custom time zones and calendars, and give details about what the default implementations of each of the optional methods do. Closes: #1040
ptomato
force-pushed
the
1040-custom-timezone-best-practice
branch
from
8563374 to
e99474d
Compare
Closes: #1040