Skip to content
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

Update as of November 2023 #296

Merged
merged 4 commits into from
Feb 27, 2025
Merged

Update as of November 2023 #296

merged 4 commits into from
Feb 27, 2025

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Feb 26, 2025

This one is very few commits but a lot of changed lines. It corresponds pretty much to tc39/proposal-temporal#2519

I'll be the first to admit that the Typescript annotations in methodrecord.ts are not very good, and there's a lot of @ts-expect-error needed to get it to compile. However, note that method records were removed from the proposal in June 2024. So I didn't want to spend a lot of time getting the types just right, since it'll be deleted in a forthcoming PR anyway.

If there are any easy-to-implement suggestions to improve methodrecord.ts I'm happy to incorporate them, but if it's complicated I'd prefer to just leave it. The sloppy code will be gone soon enough!

Introduce a new spec type, Time Zone Methods Record, which stores a time
zone object's getOffsetNanosecondsFor and getPossibleInstantsFor methods
once they have been observably looked up. The record is passed around into
abstract operations instead of the time zone object itself.

Most API operations that do time zone calculations need to look up both
getOffsetNanosecondsFor and getPossibleInstantsFor unconditionally. Only
in the case where we convert a plain time to an exact time, it's possible
we only need to look up getPossibleInstantsFor, or neither (if the offset
is supplied.)

This is a large commit but most of it is mechanical replacement of
Temporal.TimeZone variables with Time Zone Record variables.

UPSTREAM_COMMIT=fce7ec8a1fc603861f81d4a557f345af304a9d90
Introduce a new spec type, Calendar Methods Record, which stores a
calendar object's methods once they have been observably looked up. The
record is passed around into abstract operations instead of the calendar
object itself.

The mechanism doesn't currently allow for storing all Temporal.Calendar
methods, only the ones that are explicitly called internally in other
places than the Zoned and Plain types' methods of the same name, as part
of a different API call.

Some methods not included in the record are only called by the same-named
methods on other Temporal types (example, Calendar.p.dayOfWeek is only
called by ZonedDateTime.p.dayOfWeek, PlainDateTime.p.dayOfWeek, and
PlainDate.p.dayOfWeek.)

Other methods not included in the record are potentially called through
lookups in PrepareTemporalFields, so it's not possible to pre-fetch them
without making polyfilling difficult. (Example, Calendar.p.monthCode is
called in `plainYearMonth.toPlainDate(...)` by looking up the
`plainYearMonth.monthCode` property in PrepareTemporalFields. Shortcutting
that lookup would mean that PlainDate.p.monthCode could never be
polyfilled.)

This is a large commit but most of it is mechanical replacement of
Temporal.Calendar variables with Calendar Methods Record variables.

UPSTREAM_COMMIT=57ec4591ec780db0249181e8ac2beff55fe8b2c7
This is an addendum to part 3 (#2671) that removes a ZonedDateTime to
PlainDateTime conversion in one place where a fast path doesn't need the
PlainDateTime. This is observable in the case where you add or subtract
two Temporal.Durations that don't have any units higher than hours.

Credit to Anba for spotting this.

Closes: #2696

UPSTREAM_COMMIT=dce8cf42d30ca59ad6a41105d0a01096bfa659ef
UPSTREAM_COMMIT=49b3b77d5da73b7dfff629195428a827221941cc
Copy link
Contributor

@justingrant justingrant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Thanks for moving the ball fwd.

MethodName: TimeZonePrototypeKeys;
};

// We switch off type checking when accessing the cached methods on this object.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, no need to worry about making this work if you're gonna remove them soon.

@ptomato ptomato merged commit b7708de into main Feb 27, 2025
19 checks passed
@ptomato ptomato deleted the rebase branch February 27, 2025 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants