size-suggestion: Fold some withX() functions into with() functions

[justingrant]

PlainDate, PlainDateTime, and ZonedDateTime have with() functions as well as withPlainTime, withPlainDate, withCalendar, and withTiimeZone functions, each expecting a different kind of input. Should we consider only having with?

This issue is adapted from suggestions made by @FrankYFTang to help reduce the size of Temporal to address concerns from implementers (especially Android and Apple Watch) that Temporal's install size and/or in-memory size is too large.

There are three cases to consider:

• withCalendar and withTimeZone are required to ensure that changes to date/time units don't happen at the same time as the time zone or calendar. Without these methods, calls like .with({ timeZone: 'America/Los_Angeles', hour: 10}) or .with({ calendar: 'chinese', day: 5}) would be very confusing for callers because it's not clear if the result should be 10AM in the old time zone or the new time zone, or the 5th day of the month in the old calendar or the new calendar.
• withPlainTime is an ergonomic support for a very common use case, which is to set the time of a PDT or ZDT instance. For example, a store that wants to consider the time of sales before the official opening time to have happened at that opening time: const notBefore8 = zdt.hour < 8 ? wdt.withPlainTime('08:00') : zdt;. Having a single-argument method also allows a string to be used for greater convenience. The alternative to withPlainTime('08:00') is painfully verbose: with({hour: 8, minute: 0, second: 0, millisecond: 0, microsecond: 0, nanosecond: 0}). The verbose form is also error-prone, because it's easy to forget one of those five zero-valued fields and end up with a not-zeroed result. So it seems unlikely that we'd want to remove this method.
• withPlainDate, however, is less commonly used and was mostly added for consistency with withPlainTime. It seems reasonable to consider removing it to save two methods across the API.

[ptomato]

Yeah, these are just not redundant with each other even if they are similarly named.

I'd be willing to reluctantly consider removing withPlainDate but everything else is a hard no for me.

[FrankYFTang]

My origional suggestion could be found in https://docs.google.com/document/d/16Rd43XB8z6iLC2c9AfF--unDFg_t476tI1PFe7PbbcM/

would be very confusing for callers because it's not clear if the result should be
This could be addressed by an additional parameter in the option bag to indicate what kind of the variant of the with is instead of by adding a new prototype function, so instead of

a.withCalendar(x)
b.withTimeZone(y)

you can have

a.with(x, {what: 'calendar'})
b.with(y, {what: 'timeZone'})

where what need to be a better name to indicate what kind of object the with() is, just as what was indicated by the suffix in the method.

[justingrant]

I'd be willing to reluctantly consider removing withPlainDate but everything else is a hard no for me.

Agreed.

you can have

a.with(x, {what: 'calendar'})
b.with(y, {what: 'timeZone'})

We could do something like that (or require that with either includes only a time zone, only a calendar, or neither of these, but either of these options would be a lot less discoverable than the current API.

[sffc]

From a readability and ergonomics point of view, this seems harmless:

// Current:
.withPlainDate("2024-05-20")
.withPlainTime("00:00")

// Proposed:
.with(Temporal.PlainDate.from("2024-05-20"))
.with(Temporal.PlainTime.from("00:00"))

// Alternative:
.with({ date: "2024-05-20" })
.with({ time: "00:00" })

As far as withCalendar and withTimeZone, I do remember litigating the ordering question a lot, which is why we decided to have separate functions. I wouldn't be opposed to merging them back into one function but throwing an exception if you mix types of things.

// Current:
.withTimeZone("America/Chicago")
.withCalendar("buddhist")

// Proposed:
.with(Temporal.TimeZone.from("America/Chicago"))
.with(Temporal.Calendar.from("buddhist"))

// Alternative:
.with({ timeZone: "America/Chicago" })
.with({ calendar: "buddhist" })

All of the following would just throw an exception:

.with({ date: "2024-05-20", monthCode: "M06" })
.with({ time: "00:00", hour: 12 })
.with({ timeZone: "America/Chicago", hour: 12 })
.with({ calendar: "buddhist", monthCode: "M05" })

[justingrant]

merging them back into one function but throwing an exception if you mix types of things.

We could do this, but it would make a confusing API even more confusing. The current with* family of APIs took months to hash out and the final result is very IDE-discoverable and makes it really obvious to users why you can't put a calendar or a time zone in your with call: because there are dedicated APIs sitting right in your autocomplete list!

The exception approach would move this discoverability to runtime and would generate data-driven exceptions that we try to avoid Temporal-wide.

So I would not be inclined to support this change.

[gilmoreorless]

From a readability and ergonomics point of view, this seems harmless:

// Current:
.withPlainDate("2024-05-20")
.withPlainTime("00:00")

// Proposed:
.with(Temporal.PlainDate.from("2024-05-20"))
.with(Temporal.PlainTime.from("00:00"))

// Alternative:
.with({ date: "2024-05-20" })
.with({ time: "00:00" })

I think it's worth re-reading the arguments put forward in #592, where the current string syntax was proposed and discussed. .with(Temporal.PlainTime.from("00:00")) may look fine on its own, but is a big step backwards in terms of readability within the context of a lot of other API calls in a codebase.

It's also worth remembering this comment from @justingrant:

This small-seeming change may be the biggest ergonomic improvement we could have made in Temporal and it addresses the #1 feedback item: more brevity.

(Yes, I'm biased as the one who proposed the shortcut string arguments, but AFAIK Justin's comments still stand.)

[justingrant]

I agree with @gilmoreorless, especially with regards to this pattern .withPlainTime("00:00") that I've found to be very, very common in real-world Temporal code.

I think we could, if we need to, live without .withPlainDate("2024-05-20"), both because it's less common and because you can always go in the reverse direction and add a time to a date-having receiver instead of the other direction.

[sffc]

Based on today's discussion I agree that we shouldn't allow .with({ date: "2024-05-20" }) or .with({ time: "00:00" }) because they allow passing conflicting options into the options bag.

However, I'm not convinced that .withPlainDate("2024-05-20") is substantially better than .with(Temporal.PlainDate.from("2024-05-20")) (or even .with(PlainDate.from("2024-05-20")) if you import all items in the Temporal namespace).

@ptomato pointed out that there is a potential calendar conflict, but the calendar conflict already exists in .withPlainDate and should be resolved in the same way. This seems like a non-issue.

This would allow us to remove the following functions:

• zonedDateTime.withPlainTime
• zonedDateTime.withPlainDate
• datetime.withPlainTime
• datetime.withPlainDate

[justingrant]

Meeting 2024-05-23: At the upcoming June 2024 TC39 plenary meeting, we'll propose removing ZonedDateTime.p.withPlainDate and PlainDateTime.p.withPlainDate.

We think that this is OK for the following reasons:

• Changing the date of an object with a time is a relatively less common case than the reverse: setting the time of a PDT or ZDT
• They introduce the calendar conflict noted above
• There's an easy workaround which is to swap the receiver and argument and instead call plainDate.toZonedDateTime or plainDate.toPlainDateTime.

I'll open a new issue to discuss withPlainTime per @sffc's comment above.

[ptomato]

Removal of withPlainDate is implemented here.