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: Change overflow behaviour of PYM/PMD.toPlainDate #2718

Merged
merged 2 commits into from
Dec 4, 2023

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Nov 3, 2023

This changes PlainMonthDay.p.toPlainDate and PlainYearMonth.p.toPlainDate to constrain nonexistent resulting dates instead of throwing.

For example,

birthdaysThisYear = listOfBirthdays.map((pmd) =>
  pmd.toPlainDate({ year: 2023 })

The above should not throw if one of the birthdays is February 29th. That would go against Temporal's design principle of avoiding data-driven exceptions.

(If you still want the throwing behaviour, you can get it using the more verbose way, see below.)

Temporal.PlainDate.from({
  year: 2023, monthCode: pmd.monthCode, day: md.day
}, { overflow: 'reject' })

Closes: #2706

@ptomato ptomato added spec-text Specification text involved normative Would be a normative change to the proposal labels Nov 3, 2023
@ptomato ptomato added this to the Stage "3.5" milestone Nov 3, 2023
Copy link

codecov bot commented Nov 3, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (34ba424) 96.38% compared to head (724dfb6) 96.38%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2718   +/-   ##
=======================================
  Coverage   96.38%   96.38%           
=======================================
  Files          21       21           
  Lines       12444    12444           
  Branches     2250     2250           
=======================================
  Hits        11994    11994           
  Misses        395      395           
  Partials       55       55           

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

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.

👍 If we must do a (hopefully final!) normative PR, it's nice that this one is so simple. Thanks for all your're doing to get Temporal over the finish line!

@ptomato
Copy link
Collaborator Author

ptomato commented Nov 16, 2023

Tests are in tc39/test262#3954.

This changes PlainMonthDay.p.toPlainDate and PlainYearMonth.p.toPlainDate
to constrain nonexistent resulting dates instead of throwing.

For example,

    birthdaysThisYear = listOfBirthdays.map((pmd) =>
      pmd.toPlainDate({ year: 2023 })

The above should not throw if one of the birthdays is February 29th. That
would go against Temporal's design principle of avoiding data-driven
exceptions.

(If you still want the throwing behaviour, you can get it using the more
verbose way, see below.)

    Temporal.PlainDate.from({
      year: 2023, monthCode: pmd.monthCode, pmd.day
    }, { overflow: 'reject' })

Closes: #2706
@ptomato
Copy link
Collaborator Author

ptomato commented Dec 4, 2023

This change got consensus at the November 2023 TC39 meeting.

@ptomato ptomato merged commit 6e12d03 into main Dec 4, 2023
9 checks passed
@ptomato ptomato deleted the 2706-toplaindate-overflow branch December 4, 2023 23:24
ptomato added a commit that referenced this pull request Mar 20, 2024
This is to address belated feedback from Anba on #2718. The options object
previously created here is unnecessary, because 'constrain' is always the
default for this option. Not creating the options object here is
consistent with the rest of the spec text.

Closes: #2803
ptomato added a commit that referenced this pull request Apr 3, 2024
This is to address belated feedback from Anba on #2718. The options object
previously created here is unnecessary, because 'constrain' is always the
default for this option. Not creating the options object here is
consistent with the rest of the spec text.

Closes: #2803
Ms2ger pushed a commit that referenced this pull request Apr 3, 2024
This is to address belated feedback from Anba on #2718. The options object
previously created here is unnecessary, because 'constrain' is always the
default for this option. Not creating the options object here is
consistent with the rest of the spec text.

Closes: #2803
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
normative Would be a normative change to the proposal spec-text Specification text involved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cookbook recipe "Birthday in 2030" throws exception for Feb 29 in non-leap year
3 participants