Proposal-temporal: Consistent pattern for additive conversion method parameters

[justingrant]

747 (conversion method bikeshed) exposed some issues with parameters of adding-data conversion methods like MonthDay.prootype.toDate. This issue is an attempt to roll up these issues into a concise list and to propose a solution those challenges. It also made sense to roll #592 into this proposal. TL;DR - this issue contains the parts of #747 and #592 that are unrelated to method names.

Problems to Solve

• a. Avoid confusion about what the single numeric parameter means when converting from MonthDay/YearMonth to Date
• b. Support non-ISO calendar fields, e.g. MonthDay->Date conversions that add both year and era.
• c. Avoid "which goes first?" ambiguity between multiple parameters in an additive conversion method, e.g. absolute.toLocalDateTime(timeZone, calendar), monthDay.toDate(year, era), date.toLocalDateTime(timeZone, time)
• d. Support string parameters, e.g. date.toDateTime('10:00'). See #592.
• e. Make it clear when reading Temporal code what a conversion is doing. This is most problematic with toDate methods but also probably applies to toDateTime and toLocalDateTime to a lesser extent.
• f. Avoid special cases for some types that don't apply to others.
• g. (OUT OF SCOPE-- SEE #747) Avoid confusion among some developers about additive vs. reductive conversions. As I commented elsewhere, I agree that this may be a problem for some developers, but IMHO the discoverability advantage fo a single pattern outweighs the benefits of splitting the pattern.

Did I miss any?

Proposal

Here's a proposal to address (a)-(f) above while not interfering with any solution we choose for (g).

1. All additive conversion methods accept only one argument. The same pattern applies to all additive conversions to address (f).
2. The single argument can be a Temporal object, a property bag, or a string. Because these all tend to be self-describing, this avoids the ambiguity of (a) and (e) and is consistent with how from works. The property bag syntax supports (b).
3. If it's a string, then it must be passable as-is to the from method of the type that's being added. This supports (d). It also prevents the confusion in (a) because numeric scalars aren't passable to from of any Temporal type.
4. If it's a property bag, then the property names should be acceptable to with of the target type. This avoids (c).
5. If the method accepts multiple parameters but only one of them is required (e.g. Date.prototype.toLocalDateTime), then a string is accepted and will be used for the required parameter only. If multiple parameters are required (e.g. Absolute.prototype.toLocalDateTime), then only the property-bag syntax is allowed.
6. No options bags are allowed. If a developer wants custom behavior that requires options, then they should use from.
7. This pattern aligns with #720 if we choose to resurrect that proposal, meaning that conversion methods and with would accept the exact same property bags that conversion methods do, which would make it easier to refactor property-bag-using code.
8. Ergonomics of this pattern probably will encourage putting string literal values into local or global variables or constants, instead of specifying literals like 'Europe/Paris' or 'iso8601' directly as parameters. This might be a net benefit to avoid typos, but I'm not sure users will see it that way.

Examples:

// self-describing
monthDay.toDate({year: 2020});

// destructuring FTW
const {year, era} = jpnDate;
monthDay.toDate({year, era});

// can use whatever style is most convenient
date.toDateTime(time);
date.toDateTime('10:00');
date.toDateTime({hour: 10});

// 2 required parameters, so object bag required
absolute.toLocalDateTime({timeZone: 'America/Los_Angeles', calendar: 'iso8601'});

// various ways to re-use existing data
const {timeZone, calendar} = someTemplateObject;
absolute.toLocalDateTime({timeZone, calendar});
const reusable = {timeZone, calendar};
abs1.toLocalDateTime(reusable);
abs2.toLocalDateTime(reusable);
abs3.toLocalDateTime(reusable);

date.toLocalDateTime('America/Los_Angeles');
const template = date.toLocalDateTime({time: '10:00', timeZone: 'America/Los_Angeles'});  // not ergonomic!
const time = Temporal.now.time();
date.toLocalDateTime({time, timeZone: template.timeZone});

The biggest downside I see with this pattern is that it makes absolute.toLocalDateTime less ergonomic in the case where it's only being called once and literals are being used for the timezone or calendar. On the other hand, if it will be called multiple times with the same time zone and calendar, then the proposed pattern may be _more_ ergonomic because the same property bag can be re-used without the weird code of spreading a 2-element array. Also, if variables are used and named timeZone and calendar then it's about as ergonomic as the current multi-argument form.

I haven't though through this proposal fully and there may be fatal flaws. Fire away!

[sffc]

I'm +1 to this proposal in its entirely if we go with separate methods for additive conversion, barring consideration of #891.

[gibson042]

I like this except for the invented time field, which a) feels out of place, and b) runs the risk of collision and/or confusion if supplied with actual time element fields like hour/minute/etc. Just like date.toDateTime({hour: 10}) is valid, so too should date.toLocalDateTime({hour: 10, timeZone}) be—and therefore _not_ date.toLocalDateTime({time, timeZone}). Temporal.Date.prototype.toLocalDateTime is definitely the odd one out anyway, and I would also support dropping it in favor of only the two-step date.toDateTime(time).toLocalDateTime(timeZone) and inverted Temporal.LocalDateTime.from({...date, ...time, timeZone}), but I think @justingrant made a compelling argument about the conversion being sufficiently common (i.e., for intervals representing a full calendar date) to include supporting conveniences (just hopefully without breaking the other Temporal patterns).

P.S. For the same reason, absolute.toLocalDateTime({timeZone: 'America/Los_Angeles', calendar: 'iso8601'}) should also be equivalent to the more concise absolute.toLocalDateTime('America/Los_Angeles'). I think that implies Option 2 in #292.

[sffc]

One suggestion. What do you think about allowing the options to be functions, in addition to primitives and strings? For example:

date.toDateTime({
    time: (date) => { /* logic to compute a time based on the date */ },
});

The main reason I'm asking is that it would make the following call site very ergonomic.

absolute.toLocalDateTime(Temporal.now);

Both timeZone and calendar would be pulled from Temporal.now.

[ptomato]

Meeting, Sept. 18: We will do this (with the exception of allowing the options to be functions). Rationale: The concerns about the time and date fields conflicting with other properties don't apply here, they only apply to the with() method. Some months ago, we had previously said that we should accept strings in fewer places, and encourage people to use strongly typed Temporal objects instead of passing strings around. That is still our advice, but due to the overwhelming feedback we've received asking for more brevity, we now believe the ease of using string literals at call sites such as these, weighs stronger than that concern.

[justingrant]

Decision 2020-09-18: We will do this (where "this" means the proposal in the OP), regardless of any decisions around spreadability. We won't do Shane's suggestion of passing a function.

[Ms2ger]

I'm trying to figure out what this would involve, exactly.

• Date/Time toDateTime are okay AFAICT
• MonthDay toDateInYear
  
  
  
  • Drop the options argument?
  
  
  • Add string argument? No clue what kind of format, though.
  
  
• YearMonth toDateOnDay
  
  
  
  • Add support for { day }? Seems pretty pointless
  
  
• Instant toDateTime: might be removed in #1026
• toLocalDateTime: doesn't exist yet

Anything I'm missing?

[ptomato]

AFAIU we'd rename both toDateInYear() and toDateOnDay() to toDate(), remove the single number argument, and require the argument to be of the form { year: 2020 } or { day: 1 } respectively.

[sffc]

In the OP, what happens when you do

date.toDateTime({hour: 10});

My assumption is that this is shorthand for

date.toDateTime({ time: Temporal.Time.from({ hour: 10 }) });

which I believe will fill in the missing fields (minute, second, subsecond) with zeros. Is that correct?

[ptomato]

In the OP, what happens when you do date.toDateTime({hour: 10}); My assumption is that this is shorthand for date.toDateTime({ time: Temporal.Time.from({ hour: 10 }) }); which I believe will fill in the missing fields (minute, second, subsecond) with zeros. Is that correct?

According to my understanding, that's correct. (and also how it currently works)

Now that the YearMonth and MonthDay changes have landed, I believe what's left to do is the changes in

• Instant.toZonedDateTime{,ISO}
• PlainDate.toZonedDateTime
• PlainTime.toZonedDateTime

[justingrant]

Now that the YearMonth and MonthDay changes have landed, I believe what's left to do is the changes in

• Instant.toZonedDateTime
• [Plain]Date.toZonedDateTime
• [Plain]Time.toZonedDateTime

What about Instant.toZonedDateTimeISO? This only takes one parameter but I assume that {timeZone: 'Asia/Tokyo'} would also work.

[ptomato]

You're right, updated

[Ms2ger]

I also forgot to remove the options argument from MonthDay.toDate.

[ptomato]

The remaining things to do are, as far as I know:

• [x] remove options argument from PlainMonthDay.toPlainDate
• [x] documentation change for PlainDate.toZonedDateTime
• [x] documentation change for PlainTime.toZonedDateTime

is that correct?

[justingrant]

More:

• [x] TS types need to be updated for all conversion method changes DONE in #1144
• [x] Documentation change for Instant.toZonedDateTime

[justingrant]

Another change that should be rolled in: add plain prefix to date and time params of Date/Time toZonedDateTime params #1143

[justingrant]

I updated the ambiguity page code examples with the syntax from this issue and verified that all the samples work. But I ran out of time to update the docs for Instant.toZonedDateTime, PlainDate.toZonedDateTime, and PlainTime.toZonedDateTime.

@ryzokuken, @Ms2ger - if you guys have time to PR those docs changes before tomorrow's workshop, that'd be great, but if you can't that's OK too. We can always help people 1:1 in the workshop if they get confused.

BTW, the shape of the updated APIs in TS format is here in case that's useful for docs: https://github.com/tc39/proposal-temporal/pull/1144/files

[Ms2ger]

I think we're done here. Let's file followups if we missed anything.