-
Notifications
You must be signed in to change notification settings - Fork 163
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
withYear() object argument #550
Conversation
Codecov Report
@@ Coverage Diff @@
## main #550 +/- ##
=======================================
Coverage 97.53% 97.53%
=======================================
Files 17 17
Lines 4013 4019 +6
Branches 596 599 +3
=======================================
+ Hits 3914 3920 +6
Misses 97 97
Partials 2 2
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.
LGTM, thanks. Will merge once @sffc approves.
if (typeof item === 'object') { | ||
({ year } = ES.ToRecord(item, [['year']])); | ||
} else { | ||
year = ES.ToInteger(item); |
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.
We weren't calling ToInteger
before? 😳
The following steps are taken: | ||
</p> | ||
<emu-alg> | ||
1. Let _monthDay_ be the *this* value. | ||
1. Perform ? RequireInternalSlot(_monthDay_, [[InitializedTemporalMonthDay]]). | ||
1. Let _y_ be ? ToInteger(_year_). |
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.
Idk why the polyfill wasn't doing this 😅
This argument is necessary when you have a calendar where the year is not enough information to convert from MonthDay to Date. e.g.: const monthDay = Temporal.MonthDay.from({ month: 4, day: 1, calendar: 'japanese' }); const date = monthDay.toDate({ year: 18, era: "heisei" }); The existing API should continue to work unchanged, although there is one behavioural change. An object with valueOf() returning a number will no longer act as a number argument, because it will instead go down the object argument code path. See: #490
Summary of the discussion in: - #490 (comment) - #490 (comment) Closes: #490
6a4f8ff
to
cff3a54
Compare
Closes: #490