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

Normative: Calendar and TimeZone removals #2925

Merged
merged 10 commits into from
Sep 5, 2024
Merged

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Jul 27, 2024

This is the remaining followup from #2895 and #2914, implementing the conclusions from the June 2024 TC39 meeting. It is stacked on top of the editorial PR #2922.

Some editorial simplifications still to be done on this PR.

@ptomato ptomato marked this pull request as draft July 27, 2024 00:43
@ptomato ptomato force-pushed the calendar-time-zone-removals branch from 4047d55 to 493ce51 Compare July 30, 2024 00:55
@ptomato ptomato mentioned this pull request Jul 30, 2024
91 tasks
@ptomato ptomato force-pushed the calendar-time-zone-removals branch from 493ce51 to 74ec1c7 Compare August 1, 2024 04:04
@ptomato ptomato force-pushed the calendar-time-zone-removals branch from 74ec1c7 to ffa4bd6 Compare August 7, 2024 00:04
@ptomato ptomato force-pushed the calendar-time-zone-removals branch 2 times, most recently from 49b2b3e to ec7cdcb Compare August 17, 2024 00:48
ptomato added a commit that referenced this pull request Aug 20, 2024
The spec text uses ToTemporalPartialDurationRecord here, not
PrepareTemporalFields.

This is relevant because we are going to refactor PrepareTemporalFields as
part of #2925.
ptomato added a commit that referenced this pull request Aug 20, 2024
ToTemporalTimeRecord can be simplified quite a lot and doesn't need to
call the more complicated PrepareTemporalFields. This change is
unobservable, but it's preparation for refactoring PrepareTemporalFields
in #2925.
@ptomato ptomato force-pushed the calendar-time-zone-removals branch from ec7cdcb to c86abf6 Compare August 20, 2024 23:15
ptomato added a commit that referenced this pull request Aug 21, 2024
ToTemporalTimeRecord can be simplified quite a lot and doesn't need to
call the more complicated PrepareTemporalFields. This change is
unobservable, but it's preparation for refactoring PrepareTemporalFields
in #2925.
@ptomato ptomato force-pushed the calendar-time-zone-removals branch from c86abf6 to 2b549d0 Compare August 21, 2024 01:23
ptomato added a commit that referenced this pull request Aug 21, 2024
The spec text uses ToTemporalPartialDurationRecord here, not
PrepareTemporalFields.

This is relevant because we are going to refactor PrepareTemporalFields as
part of #2925.
ptomato added a commit that referenced this pull request Aug 21, 2024
ToTemporalTimeRecord can be simplified quite a lot and doesn't need to
call the more complicated PrepareTemporalFields. This change is
unobservable, but it's preparation for refactoring PrepareTemporalFields
in #2925.
ptomato added a commit to ptomato/test262 that referenced this pull request Aug 21, 2024
…objs

Tweak some tests to provide coverage of new execution paths in the spec,
such as calling GetOptionsObject inside ToTemporal___; add a few new tests
for things that weren't covered before, such as rounding a PlainDateTime
at the edge of the range; and tweak the tests verifying when the
properties of the options bag are read, which I made a mistake in tc39#4119.

See: tc39/proposal-temporal#2925
@ptomato ptomato force-pushed the calendar-time-zone-removals branch from 2b549d0 to 2fa1b21 Compare August 22, 2024 00:04
@ptomato ptomato marked this pull request as ready for review August 22, 2024 00:04
@ptomato
Copy link
Collaborator Author

ptomato commented Aug 22, 2024

This is ready for review now. There is a small amount of additional and tweaked test coverage, in tc39/test262#4206.

Copy link
Collaborator

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

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

Thank you!

spec/calendar.html Outdated Show resolved Hide resolved
ptomato added a commit to ptomato/test262 that referenced this pull request Aug 22, 2024
…objs

Tweak some tests to provide coverage of new execution paths in the spec,
such as calling GetOptionsObject inside ToTemporal___; add a few new tests
for things that weren't covered before, such as rounding a PlainDateTime
at the edge of the range; and tweak the tests verifying when the
properties of the options bag are read, which I made a mistake in tc39#4119.

See: tc39/proposal-temporal#2925
ptomato added a commit to tc39/test262 that referenced this pull request Aug 22, 2024
…objs

Tweak some tests to provide coverage of new execution paths in the spec,
such as calling GetOptionsObject inside ToTemporal___; add a few new tests
for things that weren't covered before, such as rounding a PlainDateTime
at the edge of the range; and tweak the tests verifying when the
properties of the options bag are read, which I made a mistake in #4119.

See: tc39/proposal-temporal#2925
Copy link
Collaborator

@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.

Amazing work on this. 🎉 🙏

docs/README.md Show resolved Hide resolved
docs/ambiguity.md Outdated Show resolved Hide resolved
docs/ambiguity.md Outdated Show resolved Hide resolved
docs/strings.md Outdated Show resolved Hide resolved
docs/zoneddatetime.md Show resolved Hide resolved
polyfill/lib/plaindatetime.mjs Show resolved Hide resolved
@ptomato
Copy link
Collaborator Author

ptomato commented Sep 5, 2024

I've had to roll #2924 into this PR because test262 tests were already merged for it. Without the fix in #2924, the tests won't pass in CI.

Hopefully this will fix uploading code coverage reports, at least
temporarily. (This uploader is deprecated and they want you to migrate to
the GitHub Action.)
@ptomato ptomato merged commit 2573914 into main Sep 5, 2024
5 checks passed
@ptomato ptomato deleted the calendar-time-zone-removals branch September 5, 2024 21:03
@justingrant
Copy link
Collaborator

Amazing work on this, @ptomato ! 🙏

bartlomieju pushed a commit to denoland/deno that referenced this pull request Sep 10, 2024
…25505)

Mainly I removed `Temporal.Calendar` and `Temporal.TimeZone` and
replaced them to APIs that handle calendar and timezone as strings.
tc39/proposal-temporal#2925

Related #24836
ptomato added a commit that referenced this pull request Oct 1, 2024
Similarly to the previous commit, we must avoid calling
GetUTCEpochNanoseconds with dates that are too large because it is
ill-defined in ECMA-262 what is supposed to happen. (See
tc39/ecma262#1087). Previously to PR #2925, that
could not happen because GetPossibleInstantsFor took a PlainDateTime
object, but now it takes an ISO Date-Time Record.

Note that this is editorial, because IsValidEpochNanoseconds would throw
anyway in this case even if GetUTCEpochNanoseconds was fully defined for
large inputs.
ptomato added a commit that referenced this pull request Oct 2, 2024
Similarly to the previous commit, we must avoid calling
GetUTCEpochNanoseconds with dates that are too large because it is
ill-defined in ECMA-262 what is supposed to happen. (See
tc39/ecma262#1087). Previously to PR #2925, that
could not happen because GetPossibleInstantsFor took a PlainDateTime
object, but now it takes an ISO Date-Time Record.

Note that this is editorial, because IsValidEpochNanoseconds would throw
anyway in this case even if GetUTCEpochNanoseconds was fully defined for
large inputs.
ptomato added a commit that referenced this pull request Oct 2, 2024
Similarly to the previous commit, we must avoid calling
GetUTCEpochNanoseconds with dates that are too large because it is
ill-defined in ECMA-262 what is supposed to happen. (See
tc39/ecma262#1087). Previously to PR #2925, that
could not happen because we used a PlainDateTime object in
InterpretISODateTimeOffset, but now we use an ISO Date-Time Record.

Note that this is editorial, because IsValidEpochNanoseconds would throw
anyway in this case even if GetUTCEpochNanoseconds was fully defined for
large inputs.
Ms2ger pushed a commit that referenced this pull request Oct 3, 2024
Similarly to the previous commit, we must avoid calling
GetUTCEpochNanoseconds with dates that are too large because it is
ill-defined in ECMA-262 what is supposed to happen. (See
tc39/ecma262#1087). Previously to PR #2925, that
could not happen because GetPossibleInstantsFor took a PlainDateTime
object, but now it takes an ISO Date-Time Record.

Note that this is editorial, because IsValidEpochNanoseconds would throw
anyway in this case even if GetUTCEpochNanoseconds was fully defined for
large inputs.
Ms2ger pushed a commit that referenced this pull request Oct 3, 2024
Similarly to the previous commit, we must avoid calling
GetUTCEpochNanoseconds with dates that are too large because it is
ill-defined in ECMA-262 what is supposed to happen. (See
tc39/ecma262#1087). Previously to PR #2925, that
could not happen because we used a PlainDateTime object in
InterpretISODateTimeOffset, but now we use an ISO Date-Time Record.

Note that this is editorial, because IsValidEpochNanoseconds would throw
anyway in this case even if GetUTCEpochNanoseconds was fully defined for
large inputs.
ptomato added a commit that referenced this pull request Oct 4, 2024
This is a follow-up request from Anba, to the normative change in #2925.
It moves syntactic validation of month codes and offset strings into
PrepareCalendarFields. This allows implementations to store month codes
and offset strings as integers in their equivalents of Calendar Fields
Records, instead of allocated strings.

Closes: #2962
ptomato added a commit that referenced this pull request Oct 4, 2024
This is a follow-up request from Anba, to the normative change in #2925.
It moves syntactic validation of month codes and offset strings into
PrepareCalendarFields. This allows implementations to store month codes
and offset strings as integers in their equivalents of Calendar Fields
Records, instead of allocated strings.

Closes: #2962
ptomato added a commit that referenced this pull request Oct 7, 2024
This is a follow-up request from Anba, to the normative change in #2925.
It moves syntactic validation of month codes and offset strings into
PrepareCalendarFields. This allows implementations to store month codes
and offset strings as integers in their equivalents of Calendar Fields
Records, instead of allocated strings.

Closes: #2962
ptomato added a commit that referenced this pull request Oct 7, 2024
This is a follow-up request from Anba, to the normative change in #2925.
It moves syntactic validation of month codes and offset strings into
PrepareCalendarFields. This allows implementations to store month codes
and offset strings as integers in their equivalents of Calendar Fields
Records, instead of allocated strings.

Closes: #2962
Ms2ger pushed a commit that referenced this pull request Oct 8, 2024
This is a follow-up request from Anba, to the normative change in #2925.
It moves syntactic validation of month codes and offset strings into
PrepareCalendarFields. This allows implementations to store month codes
and offset strings as integers in their equivalents of Calendar Fields
Records, instead of allocated strings.

Closes: #2962
Ms2ger pushed a commit that referenced this pull request Oct 8, 2024
This is a follow-up request from Anba, to the normative change in #2925.
It moves syntactic validation of month codes and offset strings into
PrepareCalendarFields. This allows implementations to store month codes
and offset strings as integers in their equivalents of Calendar Fields
Records, instead of allocated strings.

Closes: #2962
ptomato added a commit that referenced this pull request Oct 15, 2024
Regression from #2925. Wrong type passed to GetOffsetNanosecondsFor; we
need the result of GetPossibleEpochNanoseconds from the preceding lines.

Closes: #3010
ptomato added a commit that referenced this pull request Oct 28, 2024
Regression from #2925. Wrong type passed to GetOffsetNanosecondsFor; we
need the result of GetPossibleEpochNanoseconds from the preceding lines.

Closes: #3010
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.

5 participants