Proposal-temporal: How to handle this code? Temporal.TimeZone.from({timeZone: 'Asia/Tokyo'})

[justingrant]

Given our new pattern (#889) for conversion methods where multiple args go into a property bag, we can expect that at least some users will use this pattern in places where we currently don't expect it. Like this:

tz = Temporal.TimeZone.from({timeZone: 'Asia/Tokyo'})

The current behavior of this code is bad. It returns without an exception but will fail downstream with a confusing error if you try to use the result for anything, e.g.

tz = Temporal.TimeZone.from({timeZone: 'Asia/Tokyo'});  // expected: throw
dt = Temporal.DateTime.from('2020-10-01T00:00');
dt.toAbsolute(tz);
// => 
// timezone.mjs:101 Uncaught TypeError: this.getPossibleAbsolutesFor is not a function
//    at Object.getAbsoluteFor (timezone.mjs:101)
//    at Object.Call (Call.js:12)
//    at Object.GetTemporalAbsoluteFor (ecmascript.mjs:745)
//    at DateTime.toAbsolute (datetime.mjs:578)
//    at <anonymous>:3:4

So it's a polyfill bug that needs fixing.

That said, in our decisions around #889 and #592, we agreed that all places in Temporal that currently accept a Temporal instance would be retrofitted to also support a string or a property bag.

But we didn't discuss how to handle places in Temporal where a string and Temporal instance is currently accepted but not a property bag. For example: TimeZone.from and Calendar.from. This is essentially the inverse case of methods like plus that currently accept a property bag or an instance but not a string.

My proposal would be to support a property bag input for those places too, so there's one consistent pattern used everywhere, that anywhere you can pass a Temporal object (including TimeZone and Calendar), then a string and a property bag should be accepted. I'm not saying we should promote this syntax, only that some users will probably assume it works and I don't see a downside from having consistency everywhere.

It also mirrors what we're discussing for handling the advice we give to developers about how to handle ISO strings, e.g.

iso = '2020-09-20T12:02:15.984303069-07:00[America/Los_Angeles]';
ldt = Temporal.LocalDateTime.from(iso);
dt = Temporal.DateTime.from(iso);
date = Temporal.Date.from(iso);
tz = Temporal.TimeZone.from(iso);
fields = ldt.getFields();
dt = Temporal.DateTime.from(fields);
date = Temporal.Date.from(fields);
tz = Temporal.TimeZone.from(fields);   // why shouldn't this work too?

I'd argue that for Calendar and TimeZone, we should accept two variants of the property bag: one for id and one for what we call the field in getFields() results from other types. Examples:

Temporal.TimeZone.from({timeZone: 'Asia/Tokyo'});
Temporal.TimeZone.from({id: 'Asia/Tokyo'});
Temporal.Calendar.from({calendar: 'japanese'});
Temporal.Calendar.from({id: 'japanese'});

If the plan above doesn't have consensus, then my alternate proposal would be that passing a property bag to those methods should throw.

[ptomato]

The thing missing from the above analysis is that a custom time zone or calendar can be indistinguishable from a property bag (e.g., { id: 'mytimezone', getOffsetNanosecondsFor: () => 0 } is a custom time zone)

The currently specified behaviour is for Temporal.TimeZone.from() to return its argument unchanged if it is an object, and everything else is converted to a string and looked up to see if it matches a known ID.

I would be fine having Temporal.TimeZone.from() and Temporal.Calendar.from() take only strings. That would make it easier for people to monkeypatch them to make their own calendars globally available, which is currently a bit tricky to get right. But on the other hand, it's just as much of a design wart as the current situation. I don't think there are any good answers to this if we want to keep plain objects as custom calendars and time zones.

[ljharb]

@ptomato i would expect TimeZone.from to return a properly internal-slotted object - iow, to only return it untouched if it's a proper instance of TimeZone. Why is there a fast path for plain objects?

[justingrant]

I don't think there are any good answers to this if we want to keep plain objects as custom calendars and time zones.

Could we require custom time zones and calendars to provide some kind of tag or brand to indicate what they are? This is a very advanced use case, so asking custom calendar/timezone authors to jump through one more hoop seems reasonable.

Then any random property bag that lacked the tag could throw.

[justingrant]

Alternatively, won't custom calendars and/or timezones have at least one required method? If yes, we could test for that method and throw if it's missing.

[ljharb]

@justingrant i'd also expect TimeZone objects to have an internal slot that can be used to brand-check a TimeZone object, without needing to duck-type.

[ptomato]

It was decided in #300 that custom time zones and calendars don't have to be subclasses, they can be plain objects (and therefore calling Temporal.TimeZone.from() on them can't return an object with the TimeZone internal slot).

[ljharb]

@ptomato why can't it? It's totally fine that anything accepting a timezone also can accept a plain object, but why wouldn't the explicit method that's designed to create a TimeZone object (the way Array.from works, for example) be able to still adapt it to a TimeZone instance?

Specifically, X.from() should always return an instanceof X, that's the point of the method.

[justingrant]

FWIW, one usage of TimeZone.from is to normalize a "TimeZone-like" input into an object that you can call TimeZoneProtocol methods on from an input that may be a TimeZone instance, a custom time zone object, a timezone ID string, or (as I'm proposing in this issue) a property bag like {timeZone: 'America/Los_Angeles'}.

I don't have a strong opinion about whether custom time zones should be derived classes or plain objects, but having such a "normalize a timezone-line input" function is quite useful, not just for Temporal but also for userland code that wants to accept timezone input parameters with the same range of formats that Temporal accepts.

[ljharb]

I agree it's useful; I feel pretty strongly that it needs to return an instance of a builtin type.

[ptomato]

If we can find out a way to make that work, that's fine, but what do you think Temporal.TimeZone.from({ id: 'mytimezone', getOffsetNanosecondsFor: () => 0 }) should do?

(cc @Ms2ger who is working on making this consistent)

[ljharb]

For example, it could return a Temporal.TimeZone object that has the appropriate id, but has an own property getOffsetNanosecondsFor that shadows the prototype property? I'm sure there's more options to explore as well.

[ptomato]

How about ...

class NotBuiltinTimeZone {
  #offset;
  constructor(offset) {
    this.#offset = offset;
  }
  toString() {
    return `not-builtin-${this.#offset}`;
  }
  getOffsetNanosecondsFor() {
    return this.#offset;
  }
}

Temporal.TimeZone.from(new NotBuiltinTimeZone(3600e9))

How should that get transformed into a builtin Temporal.TimeZone? There's a way to make it work by having some sort of wrapper Temporal.TimeZone alongside the offset and named variations, but what's the benefit of adding this sort of Frankenstein object to the language? I don't see the justification for a blanket rule of "the return value of X.from() must always be an instance of X". It's a good rule in general, but it conflicts with the equally good rule that non-subclass TimeZone objects can be used everywhere that subclass TimeZone objects can be.

[ljharb]

That’s a fair point, and a great justification for simply not supporting this use case. Users can pass a string, or a plain object, or a proper TimeZone.

Why would anyone need to construct such a class and also be unable to extends TimeZone? We want Temporal to have a consistent subclassing story, unlike, say, RegExp - and imo the best of the few choices to do that is to require proper subclasses.

[ptomato]

I agree with what you say about a consistent subclassing story. I wonder if we shouldn't just remove the "plain object" kinds of time zones and calendars, and require subclassing.

I'm not sure what the motivation was for allowing plain objects in the first place, since that was before my time. It's true we do allow for example plain { year, month, day } objects in places where a Temporal.Date is expected, and feed them through Temporal.Date.from() in order to convert them, but I do think that time zones and calendars, which consist of _methods_ rather than _fields_, are different. (It's certainly not the case that if you pass a plain { year, month, day, equals() { return true; } } object where a Temporal.Date is expected, that you get a Temporal.Date instance that equals everything.)

Users can pass a string, or a plain object, or a proper TimeZone

(by "plain object" here you mean "object whose constructor property must be the builtin Object? This to me would be another good reason not to have plain-object time zones)

[ljharb]

Yes, i mean like conceptually "an options bag" or "a bag of fields", as opposed to a serialized value (string) or an actual proper TimeZone object with internal state (proper subclass).

[justingrant]

I admit I've never understood the importance of a plain-object custom timezone or calendar. I'd be in favor of dropping that requirement, but first I'd suggest that we try to figure out (from folks with a longer history than us) why this requirement got here in the first place. Philip, can we discuss at tomorrow's champions meeting?

[ryzokuken]

Is this in the agenda?

[justingrant]

Meeting 2020-10-15:

1. timeZone and calendar should be forbidden (in docs) as properties of a custom TimeZone or Calendar object, respectively.
2. from can therefore use presence of these properties to differentiate the "plain object TimeZone/Calendar" case from the {timeZone: 'America/New_York'} or {calendar: 'japanese'} cases.
   
   
   
   • 2.1 TimeZone.from({timeZone: 'Asia/Tokyo'}) will be treated the same as TimeZone.from('Asia/Tokyo').
   
   
   • 2.2 Calendar.from({calendar: 'japanese'}) will be treated the same as Calendar.from('japanese')
   
   
   • 2.3 These behaviors match how from behaves on other Temporal types
   
   
3. If TimeZone.from or Calendar.from detects a timeZone or calendar property, then it should act as if that property's value was used as the input to from. All types supported by from should be allowed: strings, TimeZone or Calendar instances, custom timezone/calendar objects, an object that's convertible to a string, or a scalar that will be converted to a string.
   
   
   
   • 3.1 Recursive property bags aren't supported. The following should throw:
   
   

TimeZone.from({timeZone: {timeZone: 'Asia/Tokyo'}});
Calendar.from({calendar: {calendar: 'japanese'}});

1. Custom TimeZone or Calendar objects can be subclasses, but per @gibson042, plain-object (non-subclassed) timezones and calendars are also allowed which enables support for cross-realm use cases.

[justingrant]

There's another problem with plain-object (non-subclassed) custom time zones: it's currently optional to implement many of TimeZone's methods. That's problematic if the custom timezone is stored on a ZonedDateTime instance because users who access the instance's timeZone property won't be able to call many TimeZone methods, including basic ones like id.

Let's discuss over in https://github.com/tc39/proposal-temporal/issues/810#issuecomment-711429469

[littledan]

I'm a bit confused by this issue. Do we really need to support this use case? I would prefer that we just interpret strings as time zones, not options bags. We don't have any other meaningful things that we want to put in this options bag, do we? I think type systems are the appropriate thing to prevent this late error from problems with duck typing; I don't think we need to make this work.

[littledan]

About https://github.com/tc39/proposal-temporal/issues/925#issuecomment-696473425 and https://github.com/tc39/proposal-temporal/issues/925#issuecomment-696478130 , I guess the way these could be unified is to say, Temporal.TimeZone.from always outputs a Temporal.TimeZone instance, but there's a separate cast algorithm which leaves things be if they are neither a string nor have a Temporal.TimeZone internal slot (with the presumption that it will meet the protocol). from can throw in this third case, providing a clean subset of the behavior.

I'm not a big fan of the conclusions in https://github.com/tc39/proposal-temporal/issues/925#issuecomment-709544520 . I apologize for not participating in this discussion earlier. As I wrote above, I don't think these options bags should be a goal. Not working to detect the options bags would simplify points 1, 2, and 3. Point 4 was already settled in #300, I thought, but I'm baffled by the "cross-realm" point--everything about internal slots already is cross-realm, so what is this referring to?