-
Notifications
You must be signed in to change notification settings - Fork 171
Polyfill: Change check to assertion in untilCalendar #3218
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
base: main
Are you sure you want to change the base?
Conversation
This check should be an assertion, as the condition is checked before calling untilCalendar.
b380ac7 to
91056e5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3218 +/- ##
==========================================
+ Coverage 96.75% 97.41% +0.65%
==========================================
Files 22 22
Lines 10398 10399 +1
Branches 1859 1826 -33
==========================================
+ Hits 10061 10130 +69
+ Misses 289 245 -44
+ Partials 48 24 -24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ptomato
left a comment
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.
I found a code snippet that triggers this assertion:
const d1 = Temporal.PlainDateTime.from('2026-01-06T11:02[u-ca=gregory]');
const d2 = Temporal.PlainDateTime.from('2026-01-07T09:02[u-ca=gregory]');
d1.until(d2, { largestUnit: 'years' });So probably we should consider it a testing coverage gap in any case.
I guess to fix this, we should decide at what level we want to have this early return. As long as it isn't observable, we can do this editorially.
One possibility is to fix all callers of CalendarDateUntil both in the polyfill and spec text so that we can make the assertion in CalendarDateUntil. Another possibility is to just make CalendarDateUntil definitively return if the two dates are equal and then we can make the assertion here in nonISOHelperBase.untilCalendar.
@ptomato I added the early return in CalendarDateUntil; that seems like the simplest thing. I'll also submit a test262 PR with your test. |
|
test262 PR: tc39/test262#4813 |
This check should be an assertion, as the condition is checked before calling untilCalendar.