Remove MonthDay.compare and add equals() methods

[ptomato]

Closes: #523

[codecov]

Codecov Report

Merging #588 into main will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #588      +/-   ##
==========================================
+ Coverage   97.77%   97.80%   +0.03%     
==========================================
  Files          16       16              
  Lines        3635     3686      +51     
  Branches      617      633      +16     
==========================================
+ Hits         3554     3605      +51     
  Misses         79       79              
  Partials        2        2              

Flag	Coverage Δ
#test262	54.17% <32.14%> (+0.03%)	⬆️
#tests	92.82% <92.20%> (+0.23%)	⬆️

Impacted Files	Coverage Δ
polyfill/lib/absolute.mjs	100.00% <100.00%> (ø)
polyfill/lib/date.mjs	93.57% <100.00%> (+0.37%)	⬆️
polyfill/lib/datetime.mjs	95.47% <100.00%> (+0.06%)	⬆️
polyfill/lib/monthday.mjs	99.00% <100.00%> (+0.04%)	⬆️
polyfill/lib/time.mjs	99.33% <100.00%> (+0.02%)	⬆️
polyfill/lib/yearmonth.mjs	97.20% <100.00%> (+0.21%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0dcb269...e574d1a. Read the comment docs.

[sffc]

I also realized that when this comes to the calendar, we may need another .equals() method there, because you can't === calendars since they aren't singletons any more.

[ljharb]

I'm confused; why would compare() === 0 not always be identical to .equals? .equals is supposed to always be derived from compare, in languages with those as protocols.

[ptomato]

By the way, equal() or equals()?

I also realized that when this comes to the calendar, we may need another .equals() method there, because you can't === calendars since they aren't singletons any more.

I didn't add one to TimeZone yet because you can do tz1.name === tz2.name, and intuition-wise, unlike the other Temporal types, I think I wouldn't expect to be able to do tz1 === tz2 on some random object and have it work.

I'm confused; why would compare() === 0 not always be identical to .equals? .equals is supposed to always be derived from compare, in languages with those as protocols.

In the absence of those precedents, I'd say this behaviour seems useful; in compare() you can compare the order of two dates, regardless of what calendar system they are in; in equal() you can tell whether two objects have the same internal state and are therefore identical. I don't think we can change compare() since a calendar can't be greater or lesser than another calendar, so if those precedents are important, then we have to change equal() to disregard the calendar.

[ljharb]

I think that the calendar system shouldn't be disregarded for compare; how else can you have a consistent comparison function?

[ptomato]

I think that the calendar system shouldn't be disregarded for compare; how else can you have a consistent comparison function?

What do you think the result of this code should be?

const today = Temporal.now.date();
Temporal.Date.compare(today.withCalendar('japanese'), today.withCalendar('hebrew'))

[ljharb]

I don't have an intuition - but compare absolutely must pick one, or else it violates the spec's requirement that sort comparators be consistent.

[sffc]

Due to the data model (#391), comparisons always take place in the ISO calendar space, no matter what is in the calendar slot. So, in .compare(), "25 Iyar 5780" == "May 19, 2 Reiwa" == "2020-05-19".

[ljharb]

In that case, compare is not useful for a consistent comparison function, which is the sole purpose of a compare method.

In other words, anything for which .compare returns 0 must be conceptually equal - .equals() must match it, in other words.

[sffc]

The way to fix that issue would be to return nonzero when the calendars differ. I don't really understand why that's important, though.

We could alternatively do string comparison on the calendar IDs to break ties.

[ljharb]

When sorting, two elements are either conceptually identical (0), or need to be sorted (negative or positive). It's fine if two conceptually identical things are observably different! However, .equals is something that's supposed to be algebraically derivable from compare.

[ptomato]

Where is this precedent coming from? There are no equal() or equals() methods anywhere in ECMAScript currently and it's not clear to me that the sole compare() method (Intl.Collator) adheres to it, since you can explicitly specify that accented letters compare equal with the sensitivity option:

new Intl.Collator('en', { sensitivity: 'base' }).compare('e', 'é')
// => 0

[ljharb]

@ptomato from other languages that have a "compare" protocol. see also this proposal on the next agenda.

[sffc]

If we're comparing to other languages, Rust uses PartialOrd and PartialEq for its == and < operators, without the requirement that the types are a perfect equivalence relation (NaN is an example they use). Rust additionally has the Ord and Eq traits for types that are full equivalence relations. The statement "a.cmp(b) == Ordering::Equal if and only if a == b and Some(a.cmp(b)) == a.partial_cmp(b) for all a and b" appears only when using Ord (equivalence relation).

[sffc]

I can see the argument that what we do here sets precedent, since as @ptomato pointed out, there aren't many other compare methods. I recall similar questions coming up when we were talking about the semantics of === on value types. If ECMA-262 wants to declare that .compare() and .equals() should follow a strict equivalence relation, then that's fine and we can make it work here (e.g., by comparing calendar IDs in addition to the ISO fields).

[sffc]

I think the comparison order should be first ISO fields, and then calendar, such that calendar is only checked if two date[time]s are otherwise identical. Probably make ISO always sort first (or last) in a list of calendars. I think this should basically cover the "ignore calendar" use case, since you get dates in their natural order, with calendar merely as a tiebreaker.

For example, an ordering would be:

1. 1900-05-20
2. 2020-05-20
3. 2020-05-20[c=hebrew]
4. 2020-05-20[c=japanese]
5. 2020-05-22
6. 2020-05-22[c=hebrew]
7. 2020-05-22[c=japanese]
8. 2020-06-01

[ptomato]

I agree calendar should be checked last, if we check it at all, but in my estimation the most common use case is going to be checking if two dates are the same date, regardless of calendar, and I think the above suggestion makes that less ergonomic: date1.withCalendar('iso8601').equals(date2.withCalendar('iso8601')).

PS. After the latest revision, this discussion no longer blocks the PR from being merged, since there is no calendar slot in the polyfill yet anyway ... 😉

[ljharb]

How can they be the same date if they're not using the same calendar?

[ptomato]

These are all the same date, not using the same calendar:

• Temporal.Date.from({ year: 2020, month: 5, day: 22, calendar: 'iso8601' })
• Temporal.Date.from({ year: 2020, month: 5, day: 22, calendar: 'gregory' })
• Temporal.Date.from({ year: 5780, month: 8, day: 28, calendar: 'hebrew' })
• Temporal.Date.from({ year: 1441, month: 9, day: 29, calendar: 'islamic' })
• Temporal.Date.from({ era: 'reiwa', year: 2, month: 5, day: 22, calendar: 'japanese' })

[ljharb]

@ptomato is that generally true? or is that just true in those instances? what i mean is, if they're the same because their ISO fields are the same, then great, that's what .equals can mean. If they're not the same because their calendars are different, then .compare shouldn't ignore the calendar when deciding to produce 0.

[ptomato]

They're the same date because they're all showing on the calendars on people's walls at the same time 😄

I've already given up on having compare() and equals() treat the calendar differently. So now the question is whether equals() and compare() both compare the calendar after comparing the ISO fields, or both ignore the calendar. If I understand correctly, either of these will fulfill your requirement.

If both methods compare the calendar after comparing the ISO fields, but you want to compare the actual dates, i.e. the dates I listed in my previous comment all compare equal, then you need to work around it like this:

• date1.withCalendar('iso8601').equals(date2.withCalendar('iso8601'))
• dates.sort((d1, d2) => Temporal.Date.compare(d1.withCalendar('iso8601'), d2.withCalendar('iso8601'))

If both methods ignore the calendar, but you want object identity, then you need to work around it like this:

• date1.equals(date2) && date1.calendar.id === date2.calendar.id
• dates.sort((d1, d2) => Temporal.Date.compare(d1, d2) || d1.calendar.id <=> d2.calendar.id) (I know there's no spaceship operator, but just to avoid cluttering it up with all the ?:)

I estimate that comparison of actual dates is the more common use case, so I'd prefer the latter workarounds to the former ones.

[ljharb]

Fair enough; I'll certainly defer to the champion group here on which alternative is better.

_{(ᕕ( ᐛ )ᕗ <=>)}

[sffc]

Can we make .compare() return -2 or +2 if the dates differ, -1 or +1 if the dates equal but the calendars differ, and 0 if the dates and calendars are both equal?

if (a.compare(b) === 0) {
  // date and calendar are both equal
}

if (Math.abs(a.compare(b)) <= 1) {
  // dates are equal, but in different calendar systems
}

FWIW, I don't find the withCalendar('iso') workaround to be painful.

[sffc]

Just an observation about #588 (comment): that example also has the correct string ordering. If we order calendar IDs alphabetically, then the ISO strings are also in lexicographic order, which seems nice.

[ptomato]

Can we make .compare() return -2 or +2 if the dates differ, -1 or +1 if the dates equal but the calendars differ, and 0 if the dates and calendars are both equal?

Sure, but I'd recommend moving that discussion to another issue. I can't implement it in this pull request because there is no calendar slot yet 😄

Is there a precedent for using specific codes in the return value of a compare-like function?

[ljharb]

.sort only cares about negative, positive, or zero. Any comparator that uses subtraction is technically providing "codes"; there's no problem with doing that, but it's not super clear to me what the value would be.

[ryzokuken]

LGTM. The champion group has agreed that it's fair to merge this in as-is for shipping the polyfill and discuss the concerns raised here before Stage 3.

[ptomato]

I opened #625 to summarize the discussion so far, and as a place to continue it.