Proposal-temporal: Era field not taken into account (bug)

[Louis-Aime]

Polyfill used 2020-11-02 and days before.
Using demonstrator localised here https://github.com/Louis-Aime/Temporal-experiences/, and directly available from https://louis-de-fouquieres.pagesperso-orange.fr/Temporal/TemporalTries.html

A Julian calendar with eras "BC" and "AD" is defined. dauc is defined as 21 April 753 B.C., the legendary date where Rom was founded.
var rauc = { era : "BC", year : 753, month : 4, day : 21, calendar : julian }, dauc = Temporal.Date.from(rauc)

dauc // => "Temporal.Date <-000752-04-13>", the right conversion of that date to iso8601
dauc.toString() // => Object { day: 21, month: 4, year: 753, era: undefined, calendar: {…} }

Other TemporalDate method, like .since and .until, do not work with "BC" dates.

[justingrant]

Hi @Louis-Aime - thanks for the feedback! The specific problem you ran into is caused by capitalization of `BC'. Examples:

date = Temporal.Date.from({ era: "bc", year: 753, month: 4, day: 21, calendar : 'gregory' });
// => -000752-04-21[c=gregory] // correct
date = Temporal.Date.from({ era: "BC", year: 753, month: 4, day: 21, calendar : 'gregory' });
// => +000753-04-21[c=gregory] // wrong

IMHO, Temporal should:

• Be lenient in parsing the era field and should accept variations in capitalization and formatting.
• Should throw a RangeError if the value isn't recognized, instead of silently treating invalid, non-undefined eras as 'ad' which is what the current code does:
  https://github.com/tc39/proposal-temporal/blob/e8dbc654525c3075ee566a11ed1eaba852fc1697/polyfill/lib/calendar.mjs#L587-L592

I'd guess that valid strings should be:

• AD: ['A.D.', 'AD', 'ad', 'a.d.', 'C.E.', 'CE', 'ce', 'c.e.']
• BC: ['B.C.', 'BC', 'bc', 'b.c.', 'B.C.E', 'BCE', 'bce', 'b.c.e.']

But @sffc will know with more clarity about what strings we should accept.

Note that regardless of the input format, the value of the property will be a lowercase 'bc' or 'ad'. These are intended as string enumerations, not user-visible values. Instead, like all localized strings in Temporal, the correct way to format a date for human consumption is to use toLocaleString(). That said, toLocaleString() also has a problem with the Gregorian calendar where the era is not shown. I filed #1100 to track this.

[sffc]

IMHO, Temporal should:

• Be lenient in parsing the era field and should accept variations in capitalization and formatting.

Mmm, I think we should be strict here.

The actual ID string, "ad" vs "AD" vs "era0" vs "gregory-era0", is a question yet to be decided by Unicode and ECMA-402 (related discussion: https://github.com/unicode-org/icu4x/issues/355). Once an ID string is chose, it should be used everywhere.

• Should throw a RangeError if the value isn't recognized, instead of silently treating invalid, non-undefined eras as 'ad' which is what the current code does:
  https://github.com/tc39/proposal-temporal/blob/e8dbc654525c3075ee566a11ed1eaba852fc1697/polyfill/lib/calendar.mjs#L587-L592

Agreed. We should throw on an invalid era code. What constitutes as "invalid" is up to the calendar implementation.

[Louis-Aime]

Thank you @justingrant . I changed to lowercase letters the era codes used in the class JulianCalendar I had defined. But things did not work at ounce. Please consider the following sequence (ran under Firefox 82.0.2 (64 bits) for Windows). Please reload before running the sequence.

dauc.getFields()
//> Object { day: 21, month: 4, year: 753, era: undefined, calendar: {…} }
dinc.withCalendar('gregory').getFields()
//> Object { day: 27, month: 8, year: 8, era: "ad", calendar: {…} }
dauc.getFields()
//> Object { day: 21, month: 4, year: 753, era: "bc", calendar: {…} }
dshak.getFields()
//> Object { day: 23, month: 4, year: 1616, era: "as", calendar: {…} }

Apparently, Temporal is not able to handle era before the code invokes 'gregory'. But after that, Temporal is able to handle era codes, even if defined in custom calendar, like "as" (for "Ancient Style") used in the custom calendar for dshak.

IHMO the 'era' custom field should work from the beginning.

[sffc]

Thank you for the feedback!

As long as the non-ISO calendars are in scope, Temporal should be equipped to handle the era field properly. If the era property in the bag of fields is not available until after you call withCalendar("gregory"), that sounds like a bug in the polyfill. @ptomato @ryzokuken, thoughts?

[ptomato]

That is the result of #1046 - the era getters are added the first time a calendar that uses era is constructed. So if the Julian calendar requires era, then it needs to add the era getters itself, as it would with any other custom field besides era. (Otherwise, the Julian calendar wouldn't work in an environment without 402 support.)

[sffc]

That is the result of #1046 - the era getters are added the first time a calendar that uses era is constructed.

IMO, the getters should be added to the prototype as soon as the calendar is loaded, not constructed.

Either way, I think the solution to @Louis-Aime's bug then is to make sure that the prototype is updated when the JulianCalendar is first used.

[Louis-Aime]

Should throw a RangeError if the value isn't recognized, instead of silently treating invalid, non-undefined eras as 'ad' which is what the current code does:
proposal-temporal/polyfill/lib/calendar.mjs
Lines 587 to 592 in e8dbc65
// Implementation details for Gregorian calendar
const gre = {
isoYear(year, era) {
return era === 'bc' ? -(year - 1) : year;
}
};

I agree. This is what I do in my experimental code. If an era is specified, it must conform to the list of era codes attached to the calendar. If no era is specified, it is up to the calendar implementation to throw an error or to guess the era from the remaining fields.

e.g. in England, the date of 19 July 1717 (where Water Music was played for the first time) was a Friday, 11 days before Monday 19 July 1717 of the Gregorian calendar. If you use the "english" calendar I have specified, this date may be specified without need to specify the era:
dwatermusic = Temporal.Date.from({ year : 1717, month : 7, day : 19, calendar : english })
The complete information is handled by the Temporal.Date object:
dwatermusic.getFields() //> Object { day: 19, month: 7, year: 1717, era: "as", calendar: {…} }, "as" meaning "Ancient Style",
dwatermusic.getISOFields() //> Object { calendar: {…}, isoDay: 30, isoMonth: 7, isoYear: 1717 }.

We can admit to leave a negative year as input fields (for Temporal.Date.from) provided that no era is indicated.