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.

Sign up for GitHub

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

Jump to bottom

Temporal.Plain{MonthDay,MonthYear}.prototype.toPlainDate: Unnecessary options object created for CalendarDateFromFields #2803

Closed
anba opened this issue Mar 11, 2024 · 3 comments · Fixed by #2806
Closed

Temporal.Plain{MonthDay,MonthYear}.prototype.toPlainDate: Unnecessary options object created for CalendarDateFromFields #2803

anba opened this issue Mar 11, 2024 · 3 comments · Fixed by #2806
Assignees
@ptomato
Labels
editorial non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved
Milestone
Stage "3.5"

Comments

@anba
Copy link
Contributor

anba commented Mar 11, 2024

Temporal.PlainMonthDay.prototype.toPlainDate:
Temporal.PlainYearMonth.prototype.toPlainDate:

  1. Let options be OrdinaryObjectCreate(null).
  2. Perform ! CreateDataPropertyOrThrow(options, "overflow", "constrain").
  3. Return ? CalendarDateFromFields(calendarRec, mergedFields, options).

It's not necessary to create an options object when "overflow" is "constrain", because that's the default overflow value.
@ptomato
Copy link
Collaborator

ptomato commented Mar 12, 2024

This is a minor oversight, but it doesn't meet the bar for a normative change at this point IMO.

@anba
Copy link
Contributor Author

anba commented Mar 12, 2024

Can this be treated as a post-commit fix for #2718?

@ptomato
Copy link
Collaborator

ptomato commented Mar 19, 2024

We discussed that at the last champions meeting and believe the process allows for that.

ptomato added a commit to ptomato/test262 that referenced this issue Mar 20, 2024
@ptomato
Temporal: Tests for calendar.dateFromFields options in PYM/PMD.toPlai…
c88eb48 
…nDate

It was previously not tested what options value a custom calendar's
dateFromFields() method would be called with, when called from the
toPlainDate() method of PlainYearMonth/PlainMonthDay.

See tc39/proposal-temporal#2803
ptomato added a commit that referenced this issue Mar 20, 2024
@ptomato
Post-adjustment: Remove unnecessary object creation
2c896b0 
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 ptomato mentioned this issue Mar 20, 2024
Merged
@ptomato ptomato self-assigned this Mar 20, 2024
@ptomato ptomato added this to the Stage "3.5" milestone Mar 20, 2024
@ptomato ptomato added spec-text Specification text involved non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! editorial labels Mar 28, 2024
Ms2ger pushed a commit to ptomato/test262 that referenced this issue Apr 3, 2024
@ptomato @Ms2ger
Temporal: Tests for calendar.dateFromFields options in PYM/PMD.toPlai…
eef1919 
…nDate

It was previously not tested what options value a custom calendar's
dateFromFields() method would be called with, when called from the
toPlainDate() method of PlainYearMonth/PlainMonthDay.

See tc39/proposal-temporal#2803
Ms2ger pushed a commit to tc39/test262 that referenced this issue Apr 3, 2024
@ptomato @Ms2ger
Temporal: Tests for calendar.dateFromFields options in PYM/PMD.toPlai…
a8da706 
…nDate

It was previously not tested what options value a custom calendar's
dateFromFields() method would be called with, when called from the
toPlainDate() method of PlainYearMonth/PlainMonthDay.

See tc39/proposal-temporal#2803
ptomato added a commit that referenced this issue Apr 3, 2024
@ptomato
Post-adjustment: Remove unnecessary object creation
d7d4414 
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 issue Apr 3, 2024
@ptomato @Ms2ger
Post-adjustment: Remove unnecessary object creation
9e7a01c 
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
Assignees

@ptomato ptomato

Labels
editorial non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved
Projects
None yet
Milestone
Stage "3.5"
Development

Successfully merging a pull request may close this issue.

Post-adjustment: Remove unnecessary object creation tc39/proposal-temporal
2 participants
@ptomato @anba