Easier way to combine multiple Temporal objects in one from or with call ?

[justingrant]

EDIT 2020-09-11: Updated this proposal to align with #747 and #592. The main changes are: a) always throwing if there's an overlap; b) allowing strings and property bags.

While adapting cookbook samples to LocalDateTime, I found it to be awkward to combine multiple Temporal objects using with() and from(). The same challenges exist with existing types like Date and DateTime so I'm filing a separate issue to discuss. Examples:

const yearMonth = Temporal.YearMonth.from('2020-11');
const time = Temporal.Time.from('01:30:00');
// "Push" Solution: chained methods
const dateTime = yearMonth.toDate(1).toDateTime(time);
// "Pull" Solution: spread properties
const dateTime = Temporal.DateTime.from({...yearMonth.getFields(), day: 1, ...time.getFields()});

After using these patterns in many cookbook samples and tests, I found the "chained methods" solution to be intuitive for doing a single concatenation but problematic when doing several at once:

• It requires learning different methods and parameter lists for each type. It'll be better with Rename conversion methods to toFoo #705, but there's still some remaining awkwardness that @ptomato is discussing over in Rename conversion methods to toFoo #705.
• It can introduce order-dependent behavior. For example, dateTime.with(yearMonth).with({day: 30}) may not return the same result as dateTime.with({day: 30}).with(yearMonth) for some dates, e.g. 2020-02-29.

The "spread properties" solution is IMHO clearer about what's going on ("assemble a new DateTime from various parts") and is immune to order dependence. I also like it because there's no new methods to learn: you just spread stuff into the object parameter. But its syntax is quite awkward. Its performance characteristics probably aren't great either.

Is there a compromise solution possible that allows adding Temporal objects to a from/with property bag without spreading? @ptomato's feedback on #700 gave me an idea: could we accept aggregate "fields" like time or date in the property bags passed to from and with methods of Date, DateTime, and LocalDateTime? Like this:

// Proposal: aggregated property bag fields
const dateTime = Temporal.DateTime.from({yearMonth, day: 1, time});
const date = Temporal.Date.from({yearMonth, day: 1});
const localDateTime = Temporal.LocalDateTime.from({yearMonth, day: 1, time, timeZone: 'Europe/Paris'});

Here's a specific proposal:

1. Date's from and with would accept yearMonth and monthDay properties.

2. DateTime's from and with would accept date, time, yearMonth, and monthDay properties.

3. LocalDateTime's from and with would accept dateTime, date, time, yearMonth, and monthDay properties.

4. These fields would not be emitted by getFields. They're only for making input methods more ergonomic.

5. If any input fields overlap (e.g. time and hour, or dayMonth and day) then throw a RangeError. In the polyfill, the error message should note the conflict, e.g. 'time' property conflicts with 'hour'.

• 5.1 Note that the overlap detection should ignore values other than undefined. If two inputs overlap and neither's value is undefined, then throw an exception even if the corresponding property values are identical. This behavior is designed to catch errors at development time instead of varying behavior at runtime. If users want overlapping, they can do the non-ergonomic approach of spreading every object.

6. As long as there's no possible overlap, these new properties would be syntactic sugar for a call to .from() whose result is spread into the input property bag. This implies that any valid argument to a type's from method should also be accepted as a value of the property for that type. Here are examples using the Time type:

• 6.1 A string:

ldt.with({ day: 1, time: '12:00' }); // proposed
ldt.with({ day: 1, ...Temporal.Time.from('12:00').getFields() });  // equivalent result

• 6.2 A Temporal instance:

ldt.with({ date: Temporal.now.date() });  // proposed
ldt.with({ day: 1, ...Temporal.now.date().getFields() });  // equivalent result

• 6.3 A property bag object:

const ldt.with({ date: Temporal.now.date() });  // proposed
ldt.with({ day: 1, ...Temporal.now.date().getFields() });  // equivalent result

7. This proposal aligns with Feedback issue for names of conversion methods #747 and Reconsider allowing strings in withDate() and withTime()? #592 so that the same fields and behavior can be used in from, with, and conversion methods like toDateTime.

Thoughts?

[ptomato]

I'm not sure the drawbacks of the chained methods are so severe as to balance adding more complexity, so my first preference would be to leave it as-is. But assuming for the moment that a change is needed, as a counter-proposal, how about accepting multiple arguments, much like in Object.assign()? That would solve the concerns about adding new property names and conflicting between fields (since the order will be well-defined).

const dateTime = Temporal.DateTime.from(yearMonth, { day: 1 }, time);
const localDateTime = Temporal.LocalDateTime.from(yearMonth, { day: 1 }, time, { timeZone: 'Europe/Paris' });

[justingrant]

how about accepting multiple arguments, much like in Object.assign()?

Won't that break the options parameter?

[ptomato]

It would require some care, but it would be possible to have the last argument be either an options bag or a fields bag.
Alternatively, the fields argument could be either a fields bag or an array of fields bags.

[justingrant]

Seems reasonable. I'd suggest an array instead of multiple args, because with a multi-arg solution IDE autocomplete would suggest options property names AND instance property names for the second and later parameters. This would degrade IDE usability for the options object in the large majority of cases where you're only passing a single property bag.

Regardless of array vs. multi-arg, IMHO the property name syntax still feels more discoverable and easier to use, esp. in an IDE where autocomplete would suggest all allowed property names. (It'd also show an overload for an array, but overloads are shown in popup help windows that are easier to ignore, while property-name autocomplete is right in the middle of the user's code so is much more discoverable.)

Also, if we did #724 (which is essentially the inverse of this suggestion) then it'd probably tip the scales in favor of property names.

[gibson042]

I've been running into this as well. Despite #403 (comment) , I don't see a clear rationale in the meeting notes for why fields must be exposed as accessors on prototypes rather than enumerable own data properties on instances (which would align with Records and trivially support naïve object spread). At the risk of arguable relitigation, I'd like to document an explicit decision on this point.

However, I don't like the redundancy and potential for inconsistency opened up by checking fields bags for composite values.

[sffc]

const date = Temporal.Date.from({yearMonth, monthDay});

I don't like this, because both yearMonth and monthDay carry a calendar. Who wins?

The status quo is fine. Either chain your methods, or spread from getFields(). If you spread, then you get the calendar from whichever spread was performed last, as defined by the spread syntax.

EDIT: Since Time doesn't carry a calendar (at least not yet), this isn't as big of a problem as I thought. I still think the status quo is fine though.

[ljharb]

If they carry different calendars, it should throw - not sure what else would make sense?

[ptomato]

I prefer the status quo. I could be convinced that an array of objects Temporal.Date.from([yearMonth, monthDay]) (note the array brackets, this is not the same as Shane's example) should be accepted, but I think it's a marginal use case.

[justingrant]

I don't see any way to defeat the issues noted above (conflicts, ordering, etc.), so my current opinion on my own suggestion is 👎. @ptomato's array suggestion could be implemented later without breaking anything, so I'd support it if we get a lot of user demand for it.

That said, if we decide to eliminate conversion methods in favor of from, then I think we'd need to revisit.

I've been running into this as well. Despite #403 (comment) , I don't see a clear rationale in the meeting notes for why fields must be exposed as accessors on prototypes rather than enumerable own data properties on instances (which would align with Records and trivially support naïve object spread). At the risk of arguable relitigation, I'd like to document an explicit decision on this point.

I agree. Requiring getFields() instead of just spreading the object feels unexpected to me, so IMHO it's worth documenting why it's needed.

[justingrant]

@gilmoreorless's proposal in #592 and the latest discussion on #747 prompted me to resurrect this issue with two significant changes that I just updated in the OP:

a) always throw if there's a possibility of a conflict. If this proposal is just an ergonomic shortcut for spreading the result of from, then it seems fine to require users with more complex use cases that need overlap to use an actual call to from.

b) per #592, we should accept strings like {time: '10:00'} both for ergonomics and to match the behavior of spreading from. This seems like a nice ergonomic win.

[ptomato]

This and #889 both leave me feeling like something is not quite right, but I'm not sure I can articulate it. Maybe the closest is Richard's remark from today's meeting about how there is no field called time in the object, but we'd be accepting one in the property bag.

[justingrant]

Are we thinking too much like physicists? We know that under the covers each of these types has slots that store each DateTime field separately. So we naturally assume that the only way to get data in or out of these types should be via those same fields. But what if we didn't know the internal data model, or if the data model of a DateTime was a Date slot and a Time slot, or if we didn't even know about getFields(). Would it still feel as weird?

Even if it is a little weird, the ergonomics & consistency benefits seem pretty large, especially now that strings can substitute for objects. Given that brevity is our top feedback, it may be worth some compromise here.

ldt.with({day:1, time: '12:00'}) 
// vs. 
ldt.with({day, Temporal.Time.from('12:00')});

BTW, here's a few mental models I've been toying with. Let me know if any of these resonate.

Possible Model 1: Expansion Aliases

I've been thinking of time or date in this context not as fields but as "aliases" or "expansions". An analogy is how microformat characters work in date-fns' format() function. An h formats the hour, m formats the minute, etc. But you can also use a p which formats the entire localized time, e.g 12:00 AM. Even though logically the h format is contained in the p format, from the perspective of format strings they are allowed as siblings.

Possible Model 2: Flattening Nested Objects

This one simply says that Temporal functions always flatten their inputs. If you pass the equivalent of (a, (b, c), d) then Temporal translates that into (a, b, c, d) and only complains if the result has duplicates like (a, b, c, c, d)

[sffc]

I think of them as expansions. ldt.with({ time: "12:00" }) is equivalent to ldt.with({ hour: 12, minute: 0, second: 0 }) (omitting fractional seconds for brevity).

If you have ldt.with({ time: "12:00", hour: 11 }), we have to decide who wins on the hour; I think the more specific field should win, so that line becomes ldt.with({ hour: 11, minute: 0, second: 0 }). Note that the minute and second implied from the time field are still used, even though the hour is overridden.

[gibson042]

If you have ldt.with({ time: "12:00", hour: 11 }), we have to decide who wins on the hour; I think the more specific field should win, so that line becomes ldt.with({ hour: 11, minute: 0, second: 0 }). Note that the minute and second implied from the time field are still used, even though the hour is overridden.

I think you picked an easy case. What about ldt.with({ time: "12:30", hour: 11 })—is that 11:00 or 11:30? It seems like someone's intuition will be off no matter how you answer, and even more so when the property order is reversed (e.g., { hour: 11, time: "12:30" }).

Then there are the ripple effects—accepting a time field for LocalDateTime implies that it should also be valid for DateTime, but what about for Time? I think Time.from("10:23:42").with({ time: "00:00" }) would surprise developers by overwriting all fields rather than just hours and minutes. And would there also be similar a date shorthand field for LocalDateTime (and for Date)?

I don't deny the ergonomic improvements, but I do question whether they outweigh the increased complexity. From my perspective, it would be a better tradeoff to instead special-case support for ldt.withTime("12:30") and possibly ldt.withDate("1955-11-05"). Replacing only date fields or only time fields should be far more common than mixing them together or mixing either with time zone and/or calendar, so I think it's acceptable to require more verbosity of those latter cases.

[justingrant]

I think you picked an easy case. What about ldt.with({ time: "12:30", hour: 11 })—is that 11:00 or 11:30? It seems like someone's intuition will be off no matter how you answer, and even more so when the property order is reversed (e.g., { hour: 11, time: "12:30" }).

Yep. This is why I proposed that we should throw if the units could possibly overlap. It's too hard to predict what the default behavior should be. If users want to handle overlaps, they can use the more complex syntax by spreading getFields() and explicitly deciding which fields should win. The time syntax is simply an ergonomic helper for the most common case where there's no ambiguity.

From my perspective, it would be a better tradeoff to instead special-case support for ldt.withTime("12:30") and possibly ldt.withDate("1955-11-05"). Replacing only date fields or only time fields should be far more common than mixing them together or mixing either with time zone and/or calendar, so I think it's acceptable to require more verbosity of those latter cases.

A problem is that withDate/ withTime would make it easier for developers to deviate from the best practice which is to make all "immutations" in one method call, because with behavior is order-dependent for any units whose length can vary. Copying from the OP:

• It can introduce order-dependent behavior. For example, dateTime.with(yearMonth).with({day: 30}) may not return the same result as dateTime.with({day: 30}).with(yearMonth) for some dates, e.g. 2020-02-29.

This is especially risky with LDT because order dependency may only show up around DST boundaries which most developers won't think to test before putting code in production.

For this reason, I believe that we shouldn't offer a variant of with that encourages developers to split into multiple with-like method calls for ergonomics reasons. Instead, I think the safer approach would be to encourage one-method-call with by making its ergonomics better. Then if you're reading code where the developer decided to split their code into multiple with calls, it's more likely that the order dependence is intentional rather than the developer simply wanting an easier codepath to write.

Another way to say it is that we should try to make "best practice" code also the easiest code to write... or at least not much harder than riskier patterns.

[gibson042]

A problem is that withDate/ withTime would make it easier for developers to deviate from the best practice which is to make all "immutations" in one method call, because with behavior is order-dependent for any units whose length can vary. Copying from the OP:

• It can introduce order-dependent behavior. For example, dateTime.with(yearMonth).with({day: 30}) may not return the same result as dateTime.with({day: 30}).with(yearMonth) for some dates, e.g. 2020-02-29.

That's true, but I don't see how it relates to the point; with remains present and chainable whether or not it accepts shorthand date/time properties and whether or not withDate and withTime are part of the API.

For this reason, I believe that we shouldn't offer a variant of with that encourages developers to split into multiple with-like method calls for ergonomics reasons. Instead, I think the safer approach would be to encourage one-method-call with by making its ergonomics better. Then if you're reading code where the developer decided to split their code into multiple with calls, it's more likely that the order dependence is intentional rather than the developer simply wanting an easier codepath to write.

Another way to say it is that we should try to make "best practice" code also the easiest code to write... or at least not much harder than riskier patterns.

Aren't mixed date/time immutations already deviating from best practices? I think "different time on the same day" is plausible (and even that is probably 80% start-of-day and 19% end-of-day), "same time on a different day" is possible, and anything else to be so rare that requiring the extra typing of ldt.with({...dateFields, ...timeFields}) seems better to me than the complexity of special-case shorthand fields in some parts of the API and throwing exceptions when they conflict with non-shorthand fields just to allow strange expressions like ldt.with({date: dateFields, time: timeFields}).

[ptomato]

Meeting, Sept. 18: We'll drop this proposal, due to the concern of date and time fields conflicting with the other fields, and general lack of agreement/support. But in order to address the underlying concern of brevity, we'll change with() methods to accept strings. We also noted a concern where passing a Temporal.Date object to with(), with a different calendar than this, would "infect" the this object with its calendar, which we've come up with a solution for. I'll close this issue and open new ones for these items.