Reconsider allowing strings in withDate() and withTime()?

[gilmoreorless]

As noted by @ptomato in #240 (comment):

There's a lot of Temporal.Something.from() involved, making for long lines. For example, to take a DateTime and get midnight on that day, it's either dateTime.getDate().withTime(Temporal.Time.from('00:00')) (or, even longer if you want to avoid the from(), dateTime.with({ hour: 0, minute: 0, second: 0, microsecond: 0, millisecond: 0, nanosecond: 0 })). It might be more readable to reverse our decision not to take strings in methods like withTime(), plus(), etc.

I agreed in a follow-up comment:

Reading all the .from() code makes the API look very cumbersome to work with, which may hinder adoption. Playing around with some use cases in the console, I found myself automatically writing things like date.withTime('10:00') and getting annoyed that it doesn't work.

From what I can tell, the decision to not allow strings in date.withTime() happened in #237, but I'm still not sure on the rationale for doing so. A lot of the cookbook examples look more complicated than they should with the current spec. I know which one of these two options I'd prefer to read in a codebase:

const date = Temporal.now.date();
date.withTime(Temporal.Time.from('09:00'));
date.withTime('09:00');

The main reason I think it should be reconsidered is that Absolute.prototype.inTimeZone() accepts a TimeZone OR a string (likewise for DateTime). So if I can write
abs.inTimeZone('Australia/Lord_Howe') instead of abs.inTimeZone(Temporal.TimeZone.from('Australia/Lord_Howe')), why can't I write date.withTime('13:45') or time.withDate('2020-11-01')?

Edit for clarity after comments: My proposal is to allow whatever is allowed in the first argument of Date.from() or Time.from(), respectively. This would include the correct Temporal objects (as it is now), strings, and property bags. Extra option arguments (e.g. overflow) would not be allowed — anyone wanting to use them can use the current technique of constructing the object via .from() directly.

[littledan]

I think TimeZones are a bit special, in terms of being treated as strings (e.g., in Intl), as well as not having any numbers/user data in them in the default case.

I've previously argued that we should be restrictive in terms of where we include "coercion" logic. I think it can get confusing if we allow lots of coercion by default in these argument positions, and that it's better to be explicit. I'm wondering, do you want options bags to be accepted in these positions too, as we previously did, before restricting coercion?

[gilmoreorless]

I'm wondering, do you want options bags to be accepted in these positions too, as we previously did, before restricting coercion?

To be honest, I hadn't thought about the options bags, mainly because I wasn't using them in my (limited) experiments. I don't see a problem with accepting them, because then it becomes easier to explain as a concept — e.g. "the argument passed to Date.prototype.withTime() is first run through Time.from()". I'd expect the same semantics and validation to apply then (i.e. if Time.from() doesn't accept it, then neither does Date.prototype.withTime()).

Note: I'm saying this purely from the point of view of an API consumer, not a spec writer. It's entirely possible there's a problematic case I've missed.

[sffc]

Strings are problematic when it comes to calendar systems, since they are unlikely to carry calendar annotations (#293). It is better to explicitly transition from strings into Temporal objects, rather than doing so implicitly in methods like Temporal.Time.prototype.withDate().

[ptomato]

I see that, but I don't necessarily agree that's a concern in the particular case of withTime and withDate. If you have date.withTime(string), then the resulting DateTime gets date's calendar. If you have time.withDate(string), then you can provide a calendar annotation in the string if we support that, and if we don't, then I wouldn't assume that an ISO string would give the resulting DateTime anything other than the ISO calendar.

[ljharb]

Also, if a string is rejected, the user's just going to make an object from the string and pass it in anyways, which doesn't provide any additional information.

[justingrant]

I support accepting strings here, both for ergonomics and for consistency with timezone or calendar parameters.

I don't think we should accept options bags in this position, because it makes for a confusing IDE autocomplete UX. Better to require undefined instead.

[gilmoreorless]

Note for context: After this issue was created, the method names were changed from .withDate() and .withTime() to .toDateTime(), but the question still stands.

I don't think we should accept options bags in this position, because it makes for a confusing IDE autocomplete UX. Better to require undefined instead.

@justingrant You've put forward the "IDE autocomplete experience" argument in a lot of cases around the API design. For the most part, I agree that we should make things easier for that scenario, and there are some nice discoverability tricks. (Your argument for .toXxx' in #747 (comment) converted me from my previous preference of date.withTime().)

However, in this case I disagree with the argument. IDE autocomplete experience is a good consideration, but should not be an overriding principle, especially when it decreases consistency in APIs.

1. Not all developers use full IDEs with language-aware autocompletion. Optimising for those who do risks becoming complacent in other areas of the API design. (e.g. "Don't worry about it being verbose, autocomplete will handle it.")

2. Being consistent is more important than someone with an IDE getting slightly confused. It's much easier to explain and understand that date.toDateTime(_thing_) is effectively just syntactic sugar for date.toDateTime(Temporal.Time.from(_thing_)).

   If a developer gets a slightly confusing result in autocomplete, they can check the documentation which explains the "syntactic sugar" concept. This won't be a new problem — I already have to check the docs for many TypeScript libraries with method overrides, because the multiple autocomplete results don't give the full story.

[justingrant]

@gilmoreorless - I'm not sure I understand our disagreement here, could you explain? Are we actually agreeing?

My point was only about options bags as related to @littledan's comment :

I'm wondering, do you want options bags to be accepted in these positions too, as we previously did, before restricting coercion?

The specific case I think should be disallowed is where the same argument position can either be a Temporal-like object literal like {hour: 12} or an options bag like {disambiguation: 'constrain'}. I think this ambiguity is unnecessarily confusing. The IDE experience is a side effect of this confusing ambiguity, because seeing disambiguation or overflow along with year, month, and day in a single autocomplete list won't help the user figure out what kind of property bag they're supposed to put in that position.

2. date.toDateTime(_thing_) is effectively just syntactic sugar for date.toDateTime(Temporal.Time.from(_thing_)).

Agreed. We don't allow options bags in the first position of from, so IMHO we shouldn't allow them in withDate/withTime either.

I also agree that withDate/withTime should accept Date-like/Time-like property bags (not options bags!) in this position, because from does too.

IDE autocomplete experience is a good consideration, but should not be an overriding principle, especially when it decreases consistency in APIs.

I also agree here. IDE experience shouldn't be paramount-- it's just one consideration among many.

OK, where do we disagree? ;-)

[gilmoreorless]

OK, where do we disagree? ;-)

Errrgh, I think this mess has come about due to my misreading of one key word. When @littledan asked about options bags, I was interpreting that as meaning property bags. 🤦

So, to clarify:

• My original thought when creating this issue was that .toDateTime() should still only take one parameter thing. The value of that is internally passed to Time.from() or Date.from() as needed.
• Therefore, the only things that should be accepted by the parameter of .toDateTime() are things that are valid for the first parameter of .from() of the respective class.
• If a user wants to provide extra disambiguation options, then that's definitely a case for using .from() manually.
• Because of this, I couldn't see where you were coming from with the autocomplete confusion. I didn't make the connection that you were talking about putting disambiguation options in the first parameter.
• Yes, we actually agree.

Sorry for all the confusion. 😄

[justingrant]

@gilmoreorless No worries, after I posted my questions above it occurred to me that you might have misread that word. All's good. Do you want to revise your OP to include both strings and object property (not options!) bags in the proposal?

The general pattern here seems to be a good one if a conversion method requires additional data, then its argument should mirror from's first parameter: a string, a Temporal class instance, or a property bag object.

This is also relevant to the discussion we're having over in #747 about conversion methods overall.

Question for you: how do you think the proposed from-like pattern should be extended when there's more than one thing required? For example: Date.prototype.toLocalDateTime which requires both a required time zone and an optional time (defaults to start of day)?

EDIT: added non-optional calendar per @sffc's latest proposal, and added Option 4 per @gibson042 suggestion in #592 (comment).

// Option 1: separate arguments
date.toLocalDateTime('America/Los_Angeles', '12:00');
date.toLocalDateTime('America/Los_Angeles');  // defaults to first moment of this day, usually but not always midnight

// Option 2: one argument: property bag with required `timeZone` property and optional `time` property
date.toLocalDateTime({timeZone: 'America/Los_Angeles', time: '12:00'});
date.toLocalDateTime({timeZone: 'America/Los_Angeles'});  // start of day, usually midnight

// Option 3: one bag with spread properties
date.toLocalDateTime({timeZone: 'America/Los_Angeles', ...Temporal.Time.from('12:00').getFields()});
date.toLocalDateTime({timeZone: 'America/Los_Angeles'});  // start of day, usually midnight

// Option 4: no conversion methods that always require multiple inputs
date.toDateTime('12:00').toLocalDateTime('America/Los_Angeles');
date.toDateTime().toLocalDateTime('America/Los_Angeles');  // start of day, usually midnight

Each of these options has pros and cons.

Option 1 is briefest and probably the most ergonomic, but it's not necessarily obvious from the API shape whether the timezone or the time should go first. This ambiguity might lead to occasional reversal bugs.

Option 2 has no ambiguity, but a time prop is most like what we decided NOT to do in #720. If we accepted that format in toDateTime/toLocalDateTime, then should it also be accepted in from? And if so, how would we deal with the possibility of conflicts which led to not adopting #720? EDIT 2020-09-11: #720 is now updated with changes that prevent conflicts.

Option 3 is, honestly, not a significant ergonomic benefit because you can't use a string for the time. It'd be easier for the caller to do this instead:

date.toDateTime('12:00').toLocalDateTime('America/Los_Angeles');

I'm not a fan of Option 4:

• It adds an extra method call for a very common use-case: getting the start of a day in a particular time zone, which is needed in every app that reads or displays UTC-timestamped data that's filtered to particular local-timezone dates. Examples include: reporting, daily schedules, local events listings by date, etc.
• It's not obviously discoverable if I am new to Temporal and don't know all the Temporal types. If I want to go from Date to LocalDateTime, it may not be obvious that I have to go through DateTime to do so-- especially depending on how we rename those two types. I'm not saying this is hard to figure out, only that it adds one more roadblock for new users.
• I'd prefer DateTime usage to be opt-in rather than incidental/accidental, because many developers who end up at DateTime may not realize that it won't adjust for DST.

I'd lean towards Option 2 if we could resurrect #720 and resolve the problems with it. Otherwise Option 1.

EDIT: one thing to keep in mind when choosing between the options above is how we'd handle MonthDay.prototype.toDate which requires a year but also might require an era or other calendar-specific properties when used with non-ISO calendars. If the pattern chosen above also works to make that case easier, that'd be ideal.

EDIT: another thing to consider: Absolute.prototype.toDateTime and Absolute.prototype.toLocalDateTime already take two parameters: a time zone and a calendar. So Temporal's conversion pattern must already deal with multiple arguments. Ideally the pattern we choose for Date=>LocalDateTime would also work for Absolute=>LocalDateTime

[gibson042]

// Option 4: no conversion methods that always require multiple inputs
date.toDateTime('12:00').toLocalDateTime('America/Los_Angeles');

[justingrant]

Yep agreed, that's another option to consider. I added it to #592 (comment) above to keep all the options in one place. I also added a variant showing how it looks when the time is set to the start-of-day default.

[justingrant]

BTW, Absolute.prototype.toLocalDateTime and Absolute.prototype.toDateTime must accept two inputs: time zone and calendar. As must MonthDay->Date conversions (whatever we call the method) which sometimes requires era or other non-ISO calendar properties. So we can't actually limit all conversion methods to one input, although in Options 2 and Option 3 above, multiple logical inputs could be combined into a single property bag.

[gilmoreorless]

Do you want to revise your OP to include both strings and object property (not options!) bags in the proposal?

Done.

Question for you: how do you think the proposed from-like pattern should be extended when there's more than one thing required? For example: Date.prototype.toLocalDateTime which requires both a required time zone and an optional time (defaults to start of day)?

My preference is Option 1 (followed by Option 2, but you're right that it causes other problems). I don't actually see the accidental reversal of arguments in Option 1 as being a big problem. Mainly because the types of arguments are different enough that reversing their order will cause parsing exceptions straight away. Though there is one case I can think of with ambiguity, when timeZone is just an offset:

date.toLocalDateTime('+10:00', '10:00') // Not identical arguments, but close enough to be confusing

[justingrant]

My preference is Option 1 (followed by Option 2, but you're right that it causes other problems).

FYI I just updated #720 which may resolve the problems with Option 2.

[justingrant]

Decision 2020-09-11:

• All places in Temporal where we accept Temporal instances and/or object bags (except perhaps relativeTo) then a string should also be accepted.
• The string should be syntactic sugar for from(someString) for the desired type.
• No options would be accepted with the string form. If users want options, they should use from.
• relativeTo is potentially problematic because it's the only place in Temporal where a string can result in different types. We'll follow up on this in Proposal: rounding and balancing for Duration type (replaces #789) #856.