-
Notifications
You must be signed in to change notification settings - Fork 155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Misc .d.ts fixes #1144
Misc .d.ts fixes #1144
Conversation
* calendar=>calendarName option for toString * updated param types for toZonedDateTime conversion methods * changed type of ZDT timeZone field to Temporal.TimeZone
Codecov Report
@@ Coverage Diff @@
## main #1144 +/- ##
==========================================
- Coverage 93.55% 90.84% -2.72%
==========================================
Files 19 17 -2
Lines 7961 8746 +785
Branches 1264 1232 -32
==========================================
+ Hits 7448 7945 +497
- Misses 506 790 +284
- Partials 7 11 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks
That doesn't sound correct? |
There was a long discussion about this at one of the meetings recently. The conclusion, assuming I'm remembering it correctly, was that 99%+ of the time, time zone usage will be built-in time zones, and 90%+ of custom time zones will be subclasses of TimeZone. And users in that remaining no-subclass custom time zone case are likely to be very sophisticated. Given that TS is essentially a type-aware linter, developers who are sophisticated enough to need a non-subclassed custom time zone are also probably sophisticated enough to override our TS declarations. So even though technically that property could not be a TimeZone, in practice it will be a TimeZone often enough that I am comfortable making our "types lint rules" reflect what almost all developers will ever see. The main reason why this is important is that if we didn't use the TimeZone type here, then developers would have to apply a type cast to every usage of many TimeZone methods that are optional on the protocol, or they'd get compiler errors. That's a pretty big usability issue. |
I think the old way is more correct and |
calendar
=>calendarName
option fortoString
timeZone
field toTemporal.TimeZone
for easier workshop coding tomorrow.