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: Reject too large dates in ToTemporalMonthDay #3054

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

anba
Copy link
Contributor

@anba anba commented Dec 9, 2024

Follow-up to #3008 / #3002 to reject too large dates.

Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.37%. Comparing base (25224a0) to head (19f17f6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3054   +/-   ##
=======================================
  Coverage   96.37%   96.37%           
=======================================
  Files          21       21           
  Lines        9874     9875    +1     
  Branches     1800     1800           
=======================================
+ Hits         9516     9517    +1     
  Misses        311      311           
  Partials       47       47           

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

@ptomato
Copy link
Collaborator

ptomato commented Dec 11, 2024

Thanks. We'll discuss this in the next champions meeting.

Copy link
Collaborator

@ptomato ptomato left a comment

Choose a reason for hiding this comment

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

Could we make the diff to be presented as minimal as possible? Ideally just the two edits in plainmonthday.html. Since the PlainYearMonth change is unobservable, I'd be happy to accept a PR for it beforehand.

ptomato pushed a commit that referenced this pull request Feb 5, 2025
…earMonth

The call to `CalendarYearMonthFromFields` will already reject too large
dates, but adding an explicit step makes it more obvious that
`ISODateToFields` doesn't require support for mapping large ISO dates to
calendar dates. And it also aligns `ToTemporalYearMonth` with
`ToTemporalMonthDay` after #3054.
Ms2ger pushed a commit that referenced this pull request Feb 5, 2025
…earMonth

The call to `CalendarYearMonthFromFields` will already reject too large
dates, but adding an explicit step makes it more obvious that
`ISODateToFields` doesn't require support for mapping large ISO dates to
calendar dates. And it also aligns `ToTemporalYearMonth` with
`ToTemporalMonthDay` after #3054.
Follow-up to tc39#3008 (tc39#3002). Similar to CalendarMonthDayToISOReferenceDate, also
don't require support for mapping ISO dates up to ±999999-12-31 to
calendar dates. This restriction does not apply to the ISO 8601
calendar.
ptomato added a commit to ptomato/test262 that referenced this pull request Feb 5, 2025
…y strings

This adds coverage for the normative change in
tc39/proposal-temporal#3054, which specifies that
implementations must throw on out-of-range dates in RFC 9557 strings for
non-ISO calendars.
@ptomato
Copy link
Collaborator

ptomato commented Feb 5, 2025

Tests are in tc39/test262#4389.

Implements the normative change from the previous commit in the reference
code.
@ptomato ptomato force-pushed the to-month-day-limit branch from 3ce9a20 to 19f17f6 Compare February 5, 2025 18:14
@ptomato ptomato marked this pull request as draft February 5, 2025 18:15
@ptomato
Copy link
Collaborator

ptomato commented Feb 5, 2025

Draft until approved in TC39.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants