-
Notifications
You must be signed in to change notification settings - Fork 28
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
Batch of rebases - early October 2023 #294
Conversation
This has been a bug in the polyfill since ZonedDateTime was originally introduced! The spec says to convert the ZonedDateTime to PlainDateTime once here, whereas the polyfill would Get each property in turn. Tests will follow as part of #2519. UPSTREAM_COMMIT=e7055b43b0af72a0439a7e6b30db5baf5457a368
Continue where #2667 left off by further clarifying this error message and fixing a mistake where `+foo` was treated as a string coercion, not the number coercion that it is. UPSTREAM_COMMIT=820fffdaf32e9c6d3fbfda6d7674d57bc58eaa38
…o ZDT We call AddZonedDateTime in several places with all of the duration components being zero except for days. In this case, we can skip the calendar operations altogether, because adding/subtracting days to/from the ISO fields is not calendar-dependent. UPSTREAM_COMMIT=ec1df1f178578eecad0b027b8c96a20d30b2c590
In the case where AddZonedDateTime is called with years, months, and weeks all zero, we can fall back to AddDaysToZonedDateTime to avoid the calendar call, and then AddInstant on the result. In BalancePossiblyInfiniteTimeDurationRelative, inline the fast path from AddZonedDateTime since we are not adding years, months, or weeks, and can now no longer call any calendar methods. Give all intermediate objects the ISO 8601 calendar for simplicity. UPSTREAM_COMMIT=5194420e7d70fa1ae67c489f3b3b84955d1b2989
…o PlainDate In a few places, we called CalendarDateAdd with a days-only duration. For days-only, it's not necessary to consult the calendar: we can just add the days in the ISO calendar space to the ISO calendar values in the internal slots. (also fixes a rebase error with truncate(_fractionalDays_)) Closes: #2685 UPSTREAM_COMMIT=1ff91a9730ace0af31177afab5c57bc343a5ab2f
Introduce an operation AddDate, which has a fast-path through the ISO calendar in the case where we would otherwise call calendar.dateAdd() with years, months, and weeks all zero. UPSTREAM_COMMIT=db1e0b33cd09284bf81fde141dec661e74844def
If two Temporal objects have identical internal slots, then we can skip calculating the difference with a calendar method; the difference is always 0. This optimization isn't necessary for PlainTime or Instant differences, since no calendar methods are called for those. Implementations can return early for PlainTime or Instant if they like, but it won't be reflected in the spec text in order to keep things as simple as possible. Note that the early return still comes after the processing of the options object. Passing an illegal options object to until() or since() should still throw, even if the difference is zero. (We'll also fast-path CalendarDateUntil, but this fast path also cuts out calls to calendar.dateFromFields().) UPSTREAM_COMMIT=27c8876007c043982201f1939a2da3e8b696237a
Introduce an operation DifferenceDate, which has a fast-path through the ISO calendar (via DaysUntil) in the case where we would otherwise call calendar.dateUntil() with largestUnit === "day". Also return a blank duration immediately if the two dates are the same day. Note that this makes it impossible for the following BalanceDuration call in DifferenceISODateTime to throw. The dateUntil() method will no longer be called for largestUnits of "day" or less. So dateDifference.[[Days]] can be at most Number.MAX_VALUE, but timeDifference cannot balance into more than one day and make it overflow in the BalanceDuration call. In NanosecondsToDays, inline the fast path from DifferenceISODateTime since we only have largestUnit === "day" there and can now no longer call any calendar methods. Give all intermediate objects the ISO 8601 calendar for simplicity. Note, as evident from the corresponding test262 tests, this removes several loopholes where it was possible to return particular values from user calls that would cause infinite loops, or calculate zero-length days. UPSTREAM_COMMIT=4bd36244f0de8420a08faa10d4614f7fe435d3a6
…mbiguous datetime In DisambiguatePossibleInstants, when getPossibleInstantsFor has returned an empty array, we calculate the UTC offset in the time zone one day before and one day after. Don't look up the method twice when doing this. Similarly, in InterpretISODateTimeOffset, when getPossibleInstantsFor has returned more than one possibility, we compare the offset nanoseconds of each possibility. Also don't look up the method twice when doing this. UPSTREAM_COMMIT=72d61c2296665aac023561ce00c6b11d6319a941
There are a few more places where we can avoid doing an additional lookup and call of getOffsetNanosecondsFor on the same ZonedDateTime, to convert it into a PlainDateTime. This affects - Temporal.Duration.prototype.add (with relativeTo ZonedDateTime) - Temporal.Duration.prototype.subtract (ditto) - Temporal.Duration.prototype.round (ditto) - Temporal.Duration.prototype.total (ditto) - Temporal.ZonedDateTime.prototype.since - Temporal.ZonedDateTime.prototype.until (also fixes "and" vs "or" prose mistakes) Closes: #2680 Closes: #2681 UPSTREAM_COMMIT=6eec81db8645823bd361310daa7b575fb702ac9f
Fix for bug in User Code Calls Part 2 (51ea969a), spotted by Anba. Thanks! The existing code forgot to take negative durations into account. Closes: #2679 UPSTREAM_COMMIT=26369ba0cf5be4b9dc8c6cecb5de16acba09e2e6
UPSTREAM_COMMIT=31a0156b7aac8e11571538d07cf4050cc55ae122
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.
Looks great!
One thing to think about is whether GetMethod
should be replaced by simple property access. Not sure it adds much value and I assume it slows down perf. But to make rebasing easier, we can save changes like this for after we're all caught up to current.
Yep, in fact I'm running into type signature problems with GetMethod a bit down the line from here. But it's not worth going through and replacing; in the current state of the Temporal proposal, GetMethod isn't used at all. |
This brings the polyfill up to date as of early October 2023. The changes are mainly around removing unnecessary user calls.