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 upstream specs #2749

Merged
merged 3 commits into from
Jan 29, 2024
Merged

Update upstream specs #2749

merged 3 commits into from
Jan 29, 2024

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Jan 12, 2024

This "rebases" Temporal on the most recent snapshots of ECMA-262 and ECMA-402, and adapts Temporal's modifications on top of them. It closes several open issues in this repo, including a few where previous "rebases" had caused unintended normative changes.

I would particularly appreciate extra review on the CreateDateTimeFormat part, because I wasn't very familiar with tc39/ecma402#709 which completely rewrote that section upstream.

Copy link

codecov bot commented Jan 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1a18bc0) 96.73% compared to head (c87871d) 96.73%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2749   +/-   ##
=======================================
  Coverage   96.73%   96.73%           
=======================================
  Files          21       21           
  Lines       12444    12444           
  Branches     2255     2255           
=======================================
  Hits        12038    12038           
  Misses        349      349           
  Partials       57       57           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ptomato
Copy link
Collaborator Author

ptomato commented Jan 16, 2024

I've pushed an update to this - I originally forgot that the operations we shared with Intl.NumberFormat v3 and Intl enumeration are now part of ECMA-402, so those need to be updated and moved into ECMA-262 as well.

Brings in the upstream ECMA-262 text as of commit tc39/ecma262@7d26449.
Copy link

@ben-allen ben-allen left a comment

Choose a reason for hiding this comment

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

This appears to me to be straightforwardly correct. My only suggestion is that there appear to be AOs with headers that explicitly list out the valid rounding mode strings rather than referring to the new Rounding modes table, and likely referring to the table is the right thing to do:

  • RoundTime
  • RoundISODateTime
  • TemporalZonedDateTimeToString

Brings in the upstream ECMA-402 text as of commit tc39/ecma402@537afda7,
and adapts existing code on top of it.

Notably, ECMA-402 has refactored so that InitializeDateTimeFormat is
replaced by CreateDateTimeFormat, and ToDateTimeOptions is removed. This
refactor changed the observable order of accessing the options passed to
the Intl.DateTimeFormat constructors, in a way that was incompatible with
changes that Temporal made to the same order of accessing. This commit
reorders the new Temporal parts of CreateDateTimeFormat so that they no
longer change the observable order of operations.

The Temporal spec text had some overlap with operations from the
Intl.NumberFormat v3 and Intl enumeration proposals. Now that these
operations are part of ECMA-402, we also recommend moving them into
ECMA-262 as part of the Temporal proposal.

Also corrects some ECMA-402 "rebase errors" that inadvertently made
normative changes in past updates.

Closes: #1932
Closes: #2363
Closes: #2473
This adds structured headers to all the new abstract operations defined
in spec/intl.html, and a Value Format Record type to describe the return
value of the HandleDateTime___ operations.
@ptomato ptomato merged commit 116788b into main Jan 29, 2024
9 checks passed
@ptomato ptomato deleted the update-upstream-specs branch January 29, 2024 21:33
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.

3 participants