Ensure reference ISO year/day is always given by the calendar #1331
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1331 +/- ##
==========================================
+ Coverage 94.82% 94.94% +0.12%
==========================================
Files 19 19
Lines 9681 9682 +1
Branches 1525 1519 -6
==========================================
+ Hits 9180 9193 +13
+ Misses 491 481 -10
+ Partials 10 8 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
In PlainYearMonth and PlainMonthDay, when constructing via the from()
method, the reference ISO day or year should always be given by the
calendar ("canonicalized") and never by user code. This was already the
case when the argument to from() is a property bag or another Temporal
object, but was not the case when the argument is a string.
This requires, slightly weirdly, calling the intrinsic PMD/PYM constructor
and then calling calendar.monthDayFromFields/yearMonthFromFields on the
result of _that_, but it is more consistent this way.
The constructor functionality remains the same, where the reference ISO
day or year is used without any change.
Closes: #1316
ptomato
force-pushed
the
1316-canonicalize-reference-year-day
branch
from
0c4b6cd
to
4e88349
Compare
| return result; | ||
|
|
||
| const PlainMonthDay = GetIntrinsic('%Temporal.PlainMonthDay%'); | ||
| let result = new PlainMonthDay(month, day, calendar, referenceISOYear); |
There was a problem hiding this comment.
In the vast majority of cases, calendar will be ISO and referenceISOYear will not be included in the string. In that case, is the double-construction needed? Seems like a worthwhile optimization to only construct twice if there's a year in the input.
Also (out of scope for this PR?), is there a case where we'll have a string like 09-04[c=japanese] where the year is omitted? Seems like that format should be disallowed; if the calendar is in the string, then a year should be required.
There was a problem hiding this comment.
I agree it's weird, and does add more observable calls to methods of calendar. I'll take a look at this tomorrow.
We do already disallow mm-dd[calendar].
Thanks for taking a look!
In PlainYearMonth and PlainMonthDay, when constructing via the from()
method, the reference ISO day or year should always be given by the
calendar ("canonicalized") and never by user code. This was already the
case when the argument to from() is a property bag or another Temporal
object, but was not the case when the argument is a string.
This requires, slightly weirdly, calling the intrinsic PMD/PYM constructor
and then calling calendar.monthDayFromFields/yearMonthFromFields on the
result of that, but it is more consistent this way.
The constructor functionality remains the same, where the reference ISO
day or year is used without any change.
Closes: #1316