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

Date[Time].from(date.getFields()) forgets the calendar #641

Closed
sffc opened this issue Jun 2, 2020 · 3 comments
Closed

Date[Time].from(date.getFields()) forgets the calendar #641

sffc opened this issue Jun 2, 2020 · 3 comments
Assignees
@ptomato
Labels
calendar Part of the effort for Temporal Calendar API
Milestone
Complete design d...

Comments

@sffc
Copy link
Collaborator

sffc commented Jun 2, 2020 •
edited
Loading

Small bug: Given the code

const d1 = Temporal.now.date().withCalendar("hebrew");
const d2 = Temporal.Date.from(d1.getFields());

d1 and d2 end up being different. d2 equals some date in Gregorian year 5780. Remedies:

  1. Ignore. This is an edge case / programmer error.
  2. Return the calendar in .getFields().
  3. Require the calendar in .from().

I have a slight preference for 2, regardless of whether we do 3.
@sffc sffc added the calendar Part of the effort for Temporal Calendar API label Jun 2, 2020
@ljharb
Copy link
Member

ljharb commented Jun 2, 2020

Option 2 seems best.

@ptomato ptomato added this to the Stable API milestone Jun 4, 2020
@ptomato
Copy link
Collaborator

ptomato commented Jun 4, 2020

This combined with what we discussed in #640 means that monthDay.with(monthDay.getFields()) will throw, but I believe that's a very minor concern, so I'm still going with option 2 in the polyfill.

ptomato added a commit that referenced this issue Jun 5, 2020
@ptomato
ISO 8601 calendar
723387f 
This adds an 'iso8601' calendar and changes the polyfill code for
Temporal.Date, Temporal.DateTime, Temporal.YearMonth, and
Temporal.MonthDay so that they gain a CALENDAR slot, and delegate all of
their calendar-sensitive operations to the object in the CALENDAR slot.

Among the changes made to those types:
- Constructors gain a calendar argument of type Temporal.Calendar.
- Types gain a 'calendar' property of type Temporal.Calendar.
- from() accepts a 'calendar' string or Temporal.Calendar.
- Temporal.Date and Temporal.DateTime accept a calendar in with().
- Temporal.MonthDay & Temporal.YearMonth throw on a calendar in with().
- The calendar is returned in getFields() (see #641).
- equals() and compare() take the calendar into account (see #625).

This includes documentation, but leaves out spec changes and all but the
necessary test changes for the time being.
ptomato added a commit that referenced this issue Jun 5, 2020
@ptomato
ISO 8601 calendar
85ca814 
This adds an 'iso8601' calendar and changes the polyfill code for
Temporal.Date, Temporal.DateTime, Temporal.YearMonth, and
Temporal.MonthDay so that they gain a CALENDAR slot, and delegate all of
their calendar-sensitive operations to the object in the CALENDAR slot.

Among the changes made to those types:
- Constructors gain a calendar argument of type Temporal.Calendar.
- Types gain a 'calendar' property of type Temporal.Calendar.
- from() accepts a 'calendar' string or Temporal.Calendar.
- Temporal.Date and Temporal.DateTime accept a calendar in with().
- Temporal.MonthDay & Temporal.YearMonth throw on a calendar in with().
- The calendar is returned in getFields() (see #641).
- equals() and compare() take the calendar into account (see #625).

This includes documentation, but leaves out spec changes and all but the
necessary test changes for the time being.
ptomato added a commit that referenced this issue Jun 9, 2020
@ptomato
ISO 8601 calendar
51f5e9b 
This adds an 'iso8601' calendar and changes the polyfill code for
Temporal.Date, Temporal.DateTime, Temporal.YearMonth, and
Temporal.MonthDay so that they gain a CALENDAR slot, and delegate all of
their calendar-sensitive operations to the object in the CALENDAR slot.

Among the changes made to those types:
- Constructors gain a calendar argument of type Temporal.Calendar.
- Types gain a 'calendar' property of type Temporal.Calendar.
- from() accepts a 'calendar' string or Temporal.Calendar.
- Temporal.Date and Temporal.DateTime accept a calendar in with().
- Temporal.MonthDay & Temporal.YearMonth throw on a calendar in with().
- The calendar is returned in getFields() (see #641).
- equals() and compare() take the calendar into account (see #625).

This includes documentation, but leaves out spec changes and all but the
necessary test changes for the time being.
ptomato added a commit that referenced this issue Jun 9, 2020
@ptomato
ISO 8601 calendar
00b5209 
This adds an 'iso8601' calendar and changes the polyfill code for
Temporal.Date, Temporal.DateTime, Temporal.YearMonth, and
Temporal.MonthDay so that they gain a CALENDAR slot, and delegate all of
their calendar-sensitive operations to the object in the CALENDAR slot.

Among the changes made to those types:
- Constructors gain a calendar argument of type Temporal.Calendar.
- Types gain a 'calendar' property of type Temporal.Calendar.
- from() accepts a 'calendar' string or Temporal.Calendar.
- Temporal.Date and Temporal.DateTime accept a calendar in with().
- Temporal.MonthDay & Temporal.YearMonth throw on a calendar in with().
- The calendar is returned in getFields() (see #641).
- equals() and compare() take the calendar into account (see #625).

This includes documentation, but leaves out spec changes and all but the
necessary test changes for the time being.
ptomato added a commit that referenced this issue Jun 10, 2020
@ptomato
ISO 8601 calendar
08a997c 
This adds an 'iso8601' calendar and changes the polyfill code for
Temporal.Date, Temporal.DateTime, Temporal.YearMonth, and
Temporal.MonthDay so that they gain a CALENDAR slot, and delegate all of
their calendar-sensitive operations to the object in the CALENDAR slot.

Among the changes made to those types:
- Constructors gain a calendar argument of type Temporal.Calendar.
- Types gain a 'calendar' property of type Temporal.Calendar.
- from() accepts a 'calendar' string or Temporal.Calendar.
- Temporal.Date and Temporal.DateTime accept a calendar in with().
- Temporal.MonthDay & Temporal.YearMonth throw on a calendar in with().
- The calendar is returned in getFields() (see #641).
- equals() and compare() take the calendar into account (see #625).

This includes documentation, but leaves out spec changes and all but the
necessary test changes for the time being.
ptomato added a commit that referenced this issue Jun 12, 2020
@ptomato
ISO 8601 calendar
3e27d67 
This adds an 'iso8601' calendar and changes the polyfill code for
Temporal.Date, Temporal.DateTime, Temporal.YearMonth, and
Temporal.MonthDay so that they gain a CALENDAR slot, and delegate all of
their calendar-sensitive operations to the object in the CALENDAR slot.

Among the changes made to those types:
- Constructors gain a calendar argument of type Temporal.Calendar.
- Types gain a 'calendar' property of type Temporal.Calendar.
- from() accepts a 'calendar' string or Temporal.Calendar.
- Temporal.Date and Temporal.DateTime accept a calendar in with().
- Temporal.MonthDay & Temporal.YearMonth throw on a calendar in with().
- The calendar is returned in getFields() (see #641).
- equals() and compare() take the calendar into account (see #625).

This includes documentation, but leaves out spec changes and all but the
necessary test changes for the time being.
@ptomato
Copy link
Collaborator

ptomato commented Jun 22, 2020

Option 2 is now the status quo, feel free to reopen this if anyone feels it needs further discussion or we need to mark it for later feedback review.

@ptomato ptomato closed this as completed Jun 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ptomato ptomato

Labels
calendar Part of the effort for Temporal Calendar API
Projects
None yet
Milestone
Complete design decisions
Development

No branches or pull requests
3 participants
@ljharb @ptomato @sffc