TimeZoneEquals should not fallback to comparison by id

[gibson042]

Raised in #2509 (comment) as commentary on a proposed change, but was observed in #2509 (comment) to be an existing issue.

TimeZoneEquals currently reports true when provided with distinct objects that have the same string value, but that behavior is problematic.

A custom time zone that happens to have id "UTC" is not equal to the built-in UTC time zone, and two custom time zones with the same id can have very different behavior.

Instead, distinct time zone objects should always be reported non-equal.

This may be in scope for #2482 .

[ljharb]

That seems like it would both block the optimization in #2482 for timezones, and also would mean that there's no reasonable way to equate two timezone objects that have the same behavior.

It seems like a timezone object that has an ID but differing behavior just has a bug.

[gibson042]

That seems like it would both block the optimization in #2482 for timezones,

No, because that optimization avoids object representations entirely when possible—the string representation is sufficient for built-in time zones, including for comparisons thereof.

and also would mean that there's no reasonable way to equate two timezone objects that have the same behavior.

That's kind of the point! Once you've got an object that can be mutated, there's no way to know which behavior is still the same. Consider code like

const tz1 = Temporal.TimeZone.from('Asia/Tokyo');
const tz2 = Temporal.TimeZone.from('Asia/Tokyo');
const zdt0 = Temporal.Now.zonedDateTimeISO();
const zdt1 = zdt0.withTimeZone(tz1);
const zdt2 = zdt0.withTimeZone(tz2);

zdt1.equals(zdt2); // first comparison
Object.defineProperties(tz2, overrides); // post-hoc mutation of id and/or methods
zdt1.equals(zdt2); // second comparison

It seems like a timezone object that has an ID but differing behavior just has a bug.

Sure, but at which level of abstraction and with how big a blast radius? I'm arguing that time zones that could behave differently from each other are never equal, which is not the case when they are the same string or the same object but is the case when they are distinct objects, and for which "id" is irrelevant.

[justingrant]

Note that the same issue affects calendars too: equals of all calendar-using types: PlainDate, PlainDateTime, ZonedDateTime, etc.

I think this issue comes down to what a true result from equals should assert.

• "As long as Temporal is not patched, it is impossible for these two objects to ever behave differently."
• "Assuming correctly-implemented custom time zones/calendars and no (or correctly-implemented) patching, these two ZDT objects will always behave the same."

I think the former guarantee would be nice to have, but I also think that the latter guarantee is OK. Developers who want an absolute guarantee that object timezones and calendars will never mess things up should probably reject all usage of non-string calendars and timezones. Treating equals more stringently than all other Temporal methods feels unnecessary.

The reason that I'm OK with #2482 is that it won't break the ability for userland code to treat string IDs and built-in TimeZone objects polymorphically. Code can take a string ID, convert it to an object using Temporal.TimeZone.from, and almost all code will continue to work, aside from rare cases like typeof or monkeypatching. Code that uses objects might be a little bit slower and might use more RAM, but ultimately the choice of string or built-in objects is purely an optimization that's almost always invisible to userland code.

But the change suggested in this issue would break this model by introducing significant functional differences. Examples:

zdt1 = Temporal.Now.zonedDateTimeISO().withTimeZone(Temporal.TimeZone.from('Asia/Tokyo'));
zdt2 = zdt1.add({ hours: 48 }).withTimeZone(Temporal.TimeZone.from('Asia/Tokyo'));

zdt.equals(zdt2); 
// => Expected: true. Actual: false

zdt.until(zdt2, { smallestUnit: 'day' }); 
// Expected: no exception. Actual: RangeError (comparing day lengths across time zones is ambiguous so not allowed)

I agree with @gibson042 that ideally there'd be a way to differentiate ZDT instances that use two custom time zones that happened to pick the same string IDs. I'd support a change that enabled this without breaking the "objects work the same as strings" behavior of the current #2482 plan.

But if the only solution to differentiating same-ID objects will break the current ability of built-in instances to be used whenever userland code uses string IDs, then I'd prefer to keep the currently-planned behavior and push the responsibilty onto custom timezone authors to use a distinct string ID for timezones that have distinct behavior. I think this aligns witih @ljharb's feedback above.

If we had more time and resources, we could extend TimeZone and the time zone protocol to add a way to evaluate whether two time zones have equivalent time zone rules, e.g. a getAllTransitions() method combined with a well-specified way to model transitions that repeat every year like DST changes. And/or a more-efficient transitionsHash property. But I'm assuming that this is out of scope for V1. And even this probably wouldn't guarantee equivalent behavior because a custom time zone could always lie about or mutate its rules.

[ptomato]

I'm fine with having this discussion separately from #2482/#1808, but I'm not in favour of considering it in scope of that change. We did already discuss this, in the context of both calendars and time zones, and the resolution was to explicitly leave it out of scope, and I'm not willing to hold up that issue to relitigate that. (Link to minutes; one of the conclusions is "If we want to change calendar equality semantics, discuss on a new issue.")

So, assuming we agree to consider it separately from #2482/#1808, here's my opinion: I'm a strong -1 on this proposal. I agree with the reasons from Jordan and Justin. But most importantly I'm afraid of this (which is the reason I gave in the linked minutes):

const calendar = Temporal.Calendar.from('gregory');
const date = Temporal.PlainDate.from({ year: 2023, month: 3, day: 2, calendar });
date.until('2023-03-21[u-ca=gregory]')

Or this:

const firstCalendar = Temporal.Calendar.from('gregory');
const firstDate = Temporal.PlainDate.from({ year: 2023, month: 3, day: 2, calendar: firstCalendar });
const secondCalendar = Temporal.Calendar.from('gregory');
const secondDate = Temporal.PlainDate.from({ year: 2023, month: 3, day: 21, calendar: secondCalendar });
firstDate.until(secondDate)

What would you expect to happen in these code snippets? With this proposed change, they would both throw a RangeError, probably with a cryptic message like "can't compare dates in 'gregory' and 'gregory' calendars." Of course we as experts know that you should use strings for your builtin calendars, for performance, but that's not obvious to learners, and I'd find it too easy to end up with Temporal objects like the above which are non-obviously 'poisoned' so they're incompatible with each other. (In the status quo, there is no problem with the above snippets.)

For the problem of calendar and time zone objects that have the same ID but different behaviour, I agree strongly with Jordan, that's a bug. I think it should be one of the invariants when creating a custom calendar or time zone that it must report a different ID if it has different behaviour, as Justin said above.

[gibson042]

I also agree strongly that distinct calendars and time zones with the same ID is a bug, and I'm very strongly against the platform masking such bugs (or worse, enabling malicious code to intentionally exploit them). I can live with allowing your zdt.until(arg) examples to pass (because the calendar and time zone associated with arg don't actually contribute any logic but are instead consulted only for string comparison against the calendar and time zone of zdt), but it should be clear that those are compatibility checks rather than equality checks and e.g. zdt.equals(zdt.withCalendar(calendarObj)) is therefore false.

[justingrant]

e.g. zdt.equals(zdt.withCalendar(calendarObj)) is therefore false.

Object equality does not guarantee equal behavior. A malicious time zone or calendar object can simply return different results from subsequent method calls.

So if we're talking accidental, non-malicious bugs, then it's worth comparing two classes of those bugs:

1. (if equals requires object equality) Bugs caused by assuming that strings and built-in tz/cal objects act the same (other than minor perf differences) across Temporal APIs
2. (if equals uses ID equality) Bugs caused by custom calendars or timezones incorrectly duplicating built-in IDs, or name collisions among custom cal/tz objects.

I assume that (1) will be vastly more common, because custom timezones and calendars are advanced, somewhat-niche features, while built-in TimeZone and Calendar objects are common. Also, we've designed the entire API surface to mask the differences between strings and objects, so having equals behave differently when no other API does will be unpredictable and confusing.

If we have to allow the possibility of bugs, I'd prefer to limit bugs to more obscure cases in exchange for mainstream cases working as expected.

[gibson042]

I don't expect any use of zdt.equals to be mainstream¹, in which case we're talking about obscure bugs that compromise integrity vs. obscure bugs that throw exceptions (highlighting their presence and thereby prompting correction).

Footnotes

1. I'm not even sure what use cases calendar-sensitive .equals are intended to support; they gratuitously diverge from static compare methods (which are not sensitive to calendar or time zone), their only use in the cookbook is as an argument for assert, and there are currently false claims there (cf. the just-opened Docs: Fix incorrect explanations and examples about ZonedDateTime #2514 ). Such surprising sensitivity may actually be worse than not having them at all, as noted in various comments of Date comparisons should not include the calendar #1431/Remove MonthDay.compare and add equals() methods #588/Comparison and equality of dates with different calendars #625/etc. ↩

[justingrant]

gratuitously diverge from static compare methods

Yeah, this one wasn't my favorite. It was a concession as a condition of getting to Stage 3. Proposal was going to be blocked otherwise, as we learned in a side meeting right before (or after?) the plenary where we asked for Stage 3. See #1431 for context. @ptomato may remember more details.

I didn't particularly agree with the request (still don't!), but I also didn't disagree so strongly that it would be worth holding up Stage 3. If I remember correctly, I think the other champions felt likewise. Consensus sometimes has its downsides.

BTW, the sort code-sample bug in your PR #2514 was caused by this change. The old behavior was to sort alphabetically by timezone and calendar ID, but the new behavior leaves ZDTs with equal timestamps sorted as-is. Clearly we forgot to fix the docs after this late-breaking change was made. Thanks for catching that!

I'm not even sure what use cases calendar-sensitive .equals are intended to support

The use case that immediately comes to mind for equals is when you are comparing an existing object (ZDT or any other Temporal type) to a list of ISO strings. For example an HTML <select> list whose options' value are ISO strings. In that case, equals can be useful to know which option should be selected.

The case for including time zone in equals seems fairly straightforward if the list represents events that happened at the same time in different places.

The case for including calendar in equals seems a bit more obscure (honestly, calendar use cases overall are pretty obscure overall), but I could imagine a calendar-comparison app with a dropdown chooser showing the same date/time in various calendars and you could use equals to know which calendar's option should be selected.

obscure bugs that throw exceptions (highlighting their presence and thereby prompting correction).

We're generally in agreement that throwing exceptions for ambiguous cases is a good thing, but I admittedly didn't know how that applies here. Sorry for not following! Could you clarify how equals fits? Are you suggesting that ZDT.p.equals should throw exceptions sometimes, e.g. when comparing two object timezones with same ID?

[gibson042]

The use case that immediately comes to mind for equals is when you are comparing an existing object (ZDT or any other Temporal type) to a list of ISO strings. For example an HTML <select> list whose options' value are ISO strings. In that case, equals can be useful to know which option should be selected.

I considered something like that, but it felt contrived to the point of uselessness... such items should either correspond with events that have their own unique identifiers in case of simultaneity, or should otherwise have an actual representation beyond just "zoned timestamp". And even if that's not the case, being explicit about including calendar and/or time zone in matching logic seems superior to hiding it inside those aspects inside a loose-string-mapping equals method, especially one whose results diverge from compare (the method which any nearest-match generalization would need to use anyway).

We're generally in agreement that throwing exceptions for ambiguous cases is a good thing, but I admittedly didn't know how that applies here. Sorry for not following! Could you clarify how equals fits? Are you suggesting that ZDT.p.equals should throw exceptions sometimes, e.g. when comparing two object timezones with same ID?

No, but to use your example of iterating over <select> options, Temporal.ZonedDateTime.from(input).withTimeZone(obj) (should it actually come up) would not match the result of parsing any existing item and would therefore hit either an explicit no-match case or fail.

[justingrant]

Meeting 2023-03-09 (forgot to record it before): We'll keep the existing behavior that ID equality is used to determine calendar equality and time zone equality.