Calendar and time zone have similar behavior. We have withCalendar but not withTimeZone. The primary reason for having withCalendar was because I believed that we wouldn't be able to come up with clean semantics on with({calendar}), but we now have that problem solved in #640. So I think we should revisit removing withCalendar, or adding withTimeZone for consistency.
If we do decide to change something then I'm more in favour of removing withCalendar than adding withTimeZone.
withTimeZone would be bad because it introduces subtle order-dependence issues. Copying from https://github.com/tc39/proposal-temporal/issues/640#issuecomment-686983494:
My strong preference is for Option 7, because it is consistent with how
with({timeZone})is handled in LocalDateTime, andwith({timeZone})is actually required for a specific case: if you're changing the time zone and other LDT fields at the same time, what you want is to apply DST disambiguation at the end after you've played the new fields on top of the new time zone. If you split up into two method calls, then you can end up with disambiguation applied to the intermediate state which may make the final state one hour different than what you expected.Also,
with({timeZone, calendar})is helpful and ergonomic. as opposed to requiring two method calls.
So no matter what, I don't think we should add withTimeZone. I'm OK with removing withCalendar to encourage developers to discover with which is useful beyond calendars.
Any objections to removing withCalendar on all types, and using with({ calendar }) instead?
Meeting, Sept. 18: We cannot remove withCalendar(), and must add withTimeZone() to LocalDateTime, to address concerns around code like dateTime.with({ ...otherDate }), which (likely unexpectedly) copies the calendar from otherDate to dateTime. Therefore the decision is that calendar and timeZone are not set by with(). (They are taken into account in case they are needed in order to know how to interpret _other_ fields of the property bag, but they don't directly affect the result.)
This change helps keep changes to time zone and calendar explicit, which is a good thing.
I started implementing this change in ZDT and now have concerns around adopting the behavior proposed above for timeZone:
Other than ZDT, there are no other Temporal types with a timezone. So (unlike calendar that's carried by dates and times) the {...date.getFields()} problematic case noted above doesn't really apply to timeZone because the only time you can get an unexpected timeZone property in the input is when spreading another ZDT, which is both rare and very obvious for the user what it will do. Does this mean that the only reason to exclude timeZone from with is for behavior consistency with calendar, not for any other reason?
After writing some sample code using the new behavior, I found it very confusing that I could include a timeZone property in the input but it didn't persist, because (again unlike calendar) the most likely case where timeZone would be present would be if I manually added it to the property bag, so by not persisting the time zone we'd likely be going against the user's explicit opt-in choice. TL;DR - if it didn't persist then it's not clear to me that there's significant value in allowing timeZone to be present in the input at all. Instead, it should probably throw or we should let timeZone persist in the result.
Let's discuss.
zdt = Temporal.ZonedDateTime.from("2020-10-01T12:00:00[America/Los_Angeles]");
zdt.with({ hour: 20, timeZone: "America/New_York" });
// zdt is now 2020-10-01T17:00:00[America/Los_Angeles]
- If we don't already do so, you should get an exception if you call .with() with a time zone and/or calendar but no other data fields. Would that help with learnability?
If we stick with the currently proposed behavior, then this seems like a good change to make.
Another compelling reason was so that you could do things like, "set the hour of this ZDT to 8pm in New York, but keep the current time zone", and also keep in mind that there was a desire to support other zoned types in user land.
IMHO after writing some code using the proposed behavior it still seemed surprising that explicitly specifying a timeZone field doesn't actually change the value of the field in this, unlike other fields like hour or day.
That said, I agree with the proposed behavior for calendar for several reasons:
calendar field will be Temporal objects, not users manually specifying a calendar field. zdt.with(time) change the calendar of zdt to ISO seems really unexpected.this from being changed _unintentionally_ seems like a good idea.this before merging into this. (I know this isn't how it works under the covers, but from a naive developer's perspective this will be how it seems to work.)TL;DR - The calendar behavior helps the ecosystem because it shields most developers from unexpected non-ISO calendar behavior and favors retaining the current calendar which is likely to be the safest behavior for the majority of developers who don't understand non-ISO calendars.
These benefits don't seem to apply (or apply much less) to time zones:
timeZone field in the input will probably come from users manually putting it there, because there's no built-in type that would sneak it in.calendar, it's reasonable to expect that a userland calendar-having type will act like Date and Time. But for timeZone there's no obvious template for users to derive their expectations from.timeZone field would act just like other fields in the with input. The same ambiguity applies to a manual calendar field added to the input, but I'd be less worried about it because a manually-added calendar field will be relatively rare because calendars usually come from other Temporal objects. (And also because most developers won't use non-ISO calendars!)So I'd lean towards:
timeZone persist (like all other non-calendar fields) OR to throw in all cases where it's provided in input, including when it's not the only field.zonedDateTIme.with(zonedDate) should not be used, either because it will throw or because it will be ambiguous.We could throw if either timeZone or calendar is present in the option bag. Plenary has been pretty clear that they don't see spreading operations as something we should encourage, so in most cases this would be a non-issue.
We could throw if either
timeZoneorcalendaris present in the option bag. Plenary has been pretty clear that they don't see spreading operations as something we should encourage, so in most cases this would be a non-issue.
Would that mean that .with(time) or with(date) would always throw if time and date were Temporal.Time or Temporal.Date instances? Or would we special-case Temporal instances and only throw for non-Temporal-instance property bags?
Meeting 2020-10-15:
withTimeZone on ZDT and keep withCalendar on ZDT and other types{} but also {calendar} or {timeZone} or {timeZone, calendar}.zdt.with(zonedTime) for a userland (or Temporal V2) ZonedTime class. A canonical use case: "What time in New York should I call my friend in London to wish him a happy new year?" Depending on the outcome of this investigation, I'll either:zonedTime's time zone but not persistedtimeZone in with.After investigating (3) above, I think we should always throw if a non-undefined timeZone property is present in the args to ZonedDateTime.prototype.with. Reasons (with details below):
with has significant issues with the canonical use case ("What time in New York should I call my friend in London to wish him a happy new year?") because of the possibility of date wrapping.A) Consistency
Elsewhere in Temporal where we throw when there's ambiguity and there's no default behavior that we think will work to resolve the ambiguity for a large majority of use cases. Here's two examples of similar cases where we also throw:
If the calendar in toLocaleString options doesn't match the calendar in the receiver, AND the receiver's calendar is non-ISO
difference when the objects have different calendars
ZonedDateTime.from when there's a conflict between offset and time zone and the default offset: 'reject' is used
ZonedDateTime.difference if largestUnit is days or larger and the time zone IDs aren't the same. Docs:
/**
* If the other `Temporal.ZonedDateTime` is in a different time zone, then the
* same days can be different lengths in each time zone, e.g. if only one of
* them observes DST. Therefore, a `RangeError` will be thrown if
* `largestUnit` is `'days'` or larger and the two instances' time zones have
* different `name` fields. To work around this limitation, transform one of
* the instances to the other's time zone using `.with({timeZone:
* other.timeZone})` and then calculate the same-timezone difference.
*/
zonedDateTime.with(zonedTime) is a similar case. It's ambiguous when reading code whether the result should have the time zone of other or this, and there's no obvious default behavior which would cover almost all use cases. Also, using with for this case seems relatively unusual compared to the preferred solution below, so it's not like we're handicapping a common case.
zonedTime.toZonedDateTime(date);
True, we could opt for consistency with calendar but most developers will never use a non-ISO calendar so this consistency won't really matter much for them because the thing it's consistent with is something they'll never notice. The 4 cases above are much more mainstream use cases, so if we want consistency, that's probably where to favor.
B) with is problematic for the canonical use case
The use case we discussed in the meeting, "What time in New York should I call my friend in London to wish him a happy new year?", isn't actually a good fit for zonedDateTime.with(zonedTime). The problem is that the date of the result is ambiguous. Unlike calendars where there's a neutral ISO calendar that can be used to evaluate any date and/or time, there's no equivalent lingua franca for time zones. For example:
zt = new ZonedTime('00:00[Europe/London]');
zdt = Temporal.now.zonedDateTimeISO('America/New_York');
callAt = zdt.with(zt); // what should be the `date` of `callAt`?
// no ambiguity via explicit conversion
callAt = zt.toZonedDateTime(zdt.date).withTimeZone(zdt.timeZone).time;
// OR (if it's before midnight where I am)
callAt = zt.toZonedDateTime(zdt.date.add({days: 1}).withTimeZone(zdt.timeZone).time;
This seems similar to the ambiguous cases listed in (A), so I think with should throw to force the user to be explicit about their intended date.
BTW, this behavior may also problematic for calendar, not for with(date) but for cases like japaneseDateTime.with({calendar: 'hebrew', month: 2}) where a partial date is provided. The result won't only change its month, it will also change the day and maybe even the year, because months are different lengths between calendars. This calendar case seems harder to resolve than timezone case where we could simply throw if a timeZone is present in the input, so we may want to try to come to consensus on the timezone case first and then figure out the harder calendar case.
Time-wrapping is a problem, but it's not specific to timeZone/calendar. Given the cyclic nature of most time units, I think it would be useful if .with() had an option like { resolution: 'nearest' | 'absolute' }, where 'nearest' goes to the nearest match, and 'absolute' sets the field directly. Like this:
let dt = Temporal.PlainDateTime.from("2020-10-01T23:00:00");
dt.with({ hour: 0, resolution: "absolute" }); // 2020-10-01T00:00:00
dt.with({ hour: 0, resolution: "nearest" }); // 2020-10-02T00:00:00
I'm not necessarily convinced by the consistency argument. Across Temporal, we generally make a best attempt effort to interpret arguments in a way that makes sense. The cases where we throw exceptions are cases where there is not a sensible interpretation.
I'm not necessarily convinced by the consistency argument. Across Temporal, we generally make a best attempt effort to interpret arguments in a way that makes sense. The cases where we throw exceptions are cases where there _is not_ a sensible interpretation.
My understanding is a little different. We've chosen to throw in cases where we think that callers are likely to assume a different default behavior, even if we could come up with a sensible default. For example, we decided to throw for code like this:
zdt.difference(zdtInOtherTimeZone, {largestUnit: 'days'})
We could have defined this method to always use this.timeZone to calculate the difference. That'd be a sensible default. But enough users might not anticipate this behavior that we opted to throw instead.
I see the zdt.with({timeZone, hour}) case as similar. It's not that we can't come up with a sensible default, it's that naive users are likely to have trouble anticipating our sensible default, which makes throwing the most sensible option. Another ingredient is whether throwing would foreclose an important ergonomic workflow. In this case it wouldn't, unless I'm missing some important use case.
Given the cyclic nature of most time units, I think it would be useful if
.with()had an option like{ resolution: 'nearest' | 'absolute' }, where'nearest'goes to the nearest match, and'absolute'sets the field directly.
I think this would be a great cookbook example. But I don't think it's necessarily wise to add this to with, because today with offers simple and easy-to-understand behavior: it only changes the properties provided in the input bag and doesn't change any others. This makes with really easy to reason about. The only possible exception I found is a rare case that may never happen in real life:
ZonedDateTime.prototype.with (other types are not affected)hour: 23 in the input to withIn this issue we're discussing whether to add another exception which is for "partial" cases like japaneseDateTime.with({calendar: 'hebrew', month: 2}) or zdt.with({timeZone: 'America/Los_Angeles', hour: 2}). If we disallow these cases then we'll retain the current simplicity of with, which I think would be best.
Also, FWIW in the past we removed options from with (e.g. the ability to balance to wrap around to a new month for example) in order to achieve with simplicity.
This is to be discussed tomorrow in the overflow meeting, but I want to emphasize that we did already reach a consensus on this, so the default will be that we stick with that decision. We can change decisions if there is new information, but if the new information is not convincing to everyone then we cannot keep discussing it endlessly.
This is to be discussed tomorrow in the overflow meeting, but I want to emphasize that we did already reach a consensus on this, so the default will be that we stick with that decision. We can change decisions if there is new information, but if the new information is not convincing to everyone then we cannot keep discussing it endlessly.
I agree, although there is actually one open question on which we've never gotten consensus: exactly how to resolve dates and times in different time zones and calendars. The current plan of action is that the timeZone and calendar properties to .with() are used to interpret the other properties in that option bag, but we haven't landed on the exact algorithm.
The algorithm I proposed was:
@pipobscure proposed instead:
This example illustrates a downside of Algorithm 1.
// 11:11pm on Thursday, October 22, in America/Denver
const a = Temporal.ZonedDateTime.from("2020-10-22T23:11:00-06:00[America/Phoenix]");
// Set hour to 8pm in America/New_York
const b = a.with({
hour: 20,
timeZone: "America/New_York",
});
Algorithm 1:
2020-10-23T01:11:00-04:00[America/New_York]2020-10-23T20:11:00-04:00[America/New_York]2020-10-23T18:11:00-06:00[America/Denver]Algorithm 2:
2020-10-22T20:00:00-04:00[America/New_York]2020-10-22T18:00:00-06:00[America/Denver]2020-10-22T18:11:00-06:00[America/Denver]Note that Algorithm 1 changes the date as well as the time.
This example illustrates a downside of Algorithm 2.
// 11:11pm on Sunday, November 1, in America/Phoenix
const a = Temporal.ZonedDateTime.from("2020-11-01T23:11:00-07:00[America/Phoenix]");
// Set hour to midnight in America/Halifax
const b = a.with({
hour: 0,
timeZone: "America/Halifax",
});
Algorithm 1:
2020-11-02T02:11:00-04:00[America/Halifax]2020-11-02T00:11:00-04:00[America/Halifax]2020-11-01T21:11:00-07:00[America/Phoenix]Algorithm 2:
2020-11-01T00:00:00-03:00[America/Halifax] (note the jump across a DST transition)2020-10-30T20:00:00-07:00[America/Phoenix]2020-11-01T20:11:00-07:00[America/Phoenix]Note that Algorithm 2 produces a ZonedDateTime that is not actually at midnight in America/Halifax due to a daylight savings time transition.
Yep, what @sffc said above was what I was (poorly) trying to convey in the meeting: that we hadn't actually made a decision on whether with can change scalar fields (i.e. not Calendar or TimeZone fields) that were not present in the input bag.
I apologize because I didn't do a good job explaining this concern in the meeting! Clearly I woke up on the wrong side of the bed this morning.
Shane, just to make sure I'm 100% understanding your points, I'll try to recapitulate what you're saying with another example. Does below accurately model the decision we have to make?
My understanding was that @sffc assumed that last week's decision was "yes, with can change other fields", while @pipobscure assumed that last week's decision was "no, with can only change scalar fields in the input". Shane and Philipp, did I understand your positions correctly?
To illustrate the difference, assume this code: _(EDIT: Earlier versions of the code below were buggy. Fixed now.)_
date = Temporal.Date.from('2020-01-01'); // in ISO calendar
date.with({ calendar: 'hebrew', month: 3 });
// FYI: the instant below in Tokyo is 3:00PM on Thursday, Jan 1 2020
zdt = Temporal.ZonedDateTime.from('2019-12-31T22:00[America/Los_Angeles]');
zdt.with( {timeZone: 'Asia/Tokyo', hour: 22 }); // same local time, but in Tokyo
Which of the following will it act like?
Option A (@pipobscure): with only changes scalar fields in its input (nothing else)
date.with({
month: date.withCalendar('hebrew').with({ month: 3 })
});
zdt.with({
hour: zdt.withTimeZone('Asia/Tokyo').with({ hour: 22 }).withTimeZone(zdt.timeZone).hour
}); // result is 5AM Weds Dec 31 2019 (17 hours earlier than original instant)
Option B (@sffc): with will change other fields too
// result may change day and year as well as month
date.withCalendar('hebrew').with({month: 3}).withCalendar('iso8601')
// result is 5AM Weds Jan 1 2020 (7 hours later than original instant)
zdt.withTimeZone('Asia/Tokyo').with({ hour: 22 }).withTimeZone('America/Los_Angeles');
Suggestion for tomorrow's discussion: before settling on a solution, could we do some quick on-screen coding of a real use case(s) for whatever solution(s) are being proposed? Having been through these problems a few times, I've consistently been surprised when solutions that seemed great on paper were problematic while coding real use cases.
Here's a few simple cases we could try:
Nothing fancy, but something like below so the plan is really precise and clear. Examples:
zdt.withTimeZone('Asia/Tokyo').with({ hour: 22 }).withTimeZone('America/Los_Angeles');
zdt.with({
hour: zdt.withTimeZone('Asia/Tokyo').with({ hour: 22 }).withTimeZone(zdt.timeZone).hour
});
Algorithm 2
@pipobscure proposed instead:
- Create a new ZonedDateTime using the argument's time zone and calendar system, filling in missing fields with default values.
- Transform the ZonedDateTime into the receiver's calendar system and time zone.
- Go back through the non-undefined fields in the argument and copy them into the receiver.
@sffc could you explain what you mean by "default values"? Is that fields from this? Or literally default values like zeroes for times, and 1 & 1 for month and day? I assumed the former, but are you assuming the latter?
@sffc could you explain what you mean by "default values"? Is that fields from
this? Or literally default values like zeroes for times, and 1 & 1 for month and day? I assumed the former, but are you assuming the latter?
My understanding from what @pipobscure said is that the date values default to the dates values from the receiver, and the time values default to zero, but @pipobscure would be better to comment on that.
@pipobscure - could you clarify what your expected output would be for the following cases?
date = Temporal.Date.from('2020-01-01'); // in ISO calendar
date.with({ calendar: 'hebrew', month: 3 });
// FYI: the instant below in India is 12:15PM on Thursday, Jan 1 2020
zdt = Temporal.ZonedDateTime.from('2019-12-31T22:45[America/Los_Angeles]');
zdt.with({ timeZone: 'Asia/Kolkata', hour: 22 });
// EDIT: if behavior differs between time units and date units, then how will be following code behave?
zdt.with({ timeZone: 'Asia/Kolkata', month: 3, hour: 22 });
zdt.with({ timeZone: 'Asia/Kolkata', month: 3 });
Sounds like there are (at least) four different options to consider. Here's the options as I understand them. If folks have time before tomorrow's meeting, could you weigh in with your preferred option? (I'd hoped to be able to get to this earlier so we wouldn't need meeting time for this, but it's been a busy week.)
date = Temporal.Date.from('2020-01-01'); // in ISO calendar
date.with({ calendar: 'hebrew', month: 3 });
// FYI: the instant below in India is 12:15PM on Thursday, Jan 1 2020
zdt = Temporal.ZonedDateTime.from('2019-12-31T22:45[America/Los_Angeles]');
zdt.with( {timeZone: 'Asia/Kolkata', hour: 22 });
// These next 2 can be ignored unless the option has different behavior for time units vs. date units
zdt.with( {timeZone: 'Asia/Kolkata', month: 3, hour: 22 });
zdt.with( {timeZone: 'Asia/Kolkata', month: 3 });
Option A) Replace only fields that are present in the input
// Only month field will change.
// Is the result meaningful, given that Hebrew & ISO months don't start on the same ISO dates?
date.with({
month: date.withCalendar('hebrew').with({ month: 3 }).withCalendar('iso8601').month
});
// Result is 14 hours earlier (same calendar day) than original instant
zdt.with({
hour: zdt.withTimeZone('Asia/Kolkata').with({ hour: 22 }).withTimeZone(zdt.timeZone).hour
}); // => 2019-12-31T08:45-08:00[America/Los_Angeles]
Option B) Update any changed fields, including fields not present in the input
// Result may change day and year as well as month
date.withCalendar('hebrew').with({month: 3}).withCalendar('iso8601')
// Result is 10 hours later (next calendar day) than original instant
zdt.withTimeZone('Asia/Kolkata').with({ hour: 22 }).withTimeZone('America/Los_Angeles');
// => 2020-01-01T08:45-08:00[America/Los_Angeles]
Option C) Default time units to zero (but only if 1+ time units are present in the input?)
I took a guess below for time zone behavior in the simplest case below, but this option needs clarification from @pipobscure about expected behavior. Specifically:
{month: 3, hour: 22, timeZone: 'Asia/Kolkata'}{month: 3}? Will it also set time units to zero?calendar in the input?// Needs clarification about how this should behave
// date.withCalendar('hebrew').with({month: 3});
// TODO: show behavior
// result is 14 hours and 15 minutes earlier (same calendar day) than original instant
intermediate = zdt.withTimeZone('Asia/Kolkata')
.with({ hour: 22, minute: 0, second: 0, millisecond: 0, microsecond: 0, nanosecond: 0 })
.withTimeZone(zdt.timeZone);
const {hour, minute, second, millisecond, microsecond, nanosecond} = intermediate.getFields();
zdt.with({hour, minute, second, millisecond, microsecond, nanosecond});
// => 2019-12-31T08:30-08:00[America/Los_Angeles]
// If behavior differs between time units and date units, then how will be following code behave?
// zdt.with({ timeZone: 'Asia/Kolkata', month: 3, hour: 22 });
// TODO: show behavior
// zdt.with({ timeZone: 'Asia/Kolkata', month: 3 });
// TODO: show behavior
Option D) Throw if calendar and/or time zone doesn't match this
with would throw if either of the following are true:
calendar property that's different from this.calendartimeZone property that's different from this.timeZoneTo use with across calendars and/or time zones, the caller must be explicit about their intent. Either they can convert this or other to the opposite calendar and/or time zone, or they can remove calendar and timeZone from the input.
The same behavior exists elsewhere in Temporal where we throw if there's a potentially confusing interaction across calendars and/or time zones:
until or since (previously called difference) when the objects have different calendars, or (ZDT only) if largestUnit is days or larger and the time zones are different.toLocaleString options doesn't match the calendar in the receiver, AND the receiver's calendar is non-ISOZonedDateTime.from when there's a conflict between offset and time zone_(EDIT: we captured the options below in the meeting discussion, so adding them here to close the loop)_
Option E) Throw if calendar and/or time zone are present in the input
Same as (D), except with would throw if calendar or timeZone is present in the input, regardless of the current timeZone or calendar properties of `this.
Option F) Throw if time zone doesn't match this or if calendar.id's don't match and both are non-ISO
Same as (D), except unmatched calendars wouldn't throw if one of them were ISO.
Option G) calendar and/or timeZone in the input act like withCalendar and/or withTimeZone, respectively
This was the previous plan of record before we started this GH issue: if a timeZone and/or calendar is present in the input, then the result will have that calendar and/or time zone. The challenge with this plan is that it works fine for time zones, but for calendars it makes it too easy to inadvertently change the calendar of this, e.g. zdt.with('10:00') would change the calendar of ZDT to ISO which is probably unexpected.
// result has a new calendar. Day and year are unchanged after the calendar is changed.
date.withCalendar('hebrew').with ({ month: 3 });
// Result is in a new time zone and is is 10 hours later than original instant
zdt.withTimeZone('Asia/Kolkata').with({ hour: 22 });
// => 2020-01-01T22:15+05:30[Asia/Calcutta]
My preference is (D) because:
with without timezone/calendar fields, which is that only the fields in the input are changed in the result while other fields are untouched.Meeting 2020-10-29:
this as a reference point, but you are OK with the result having meaningless values for any units that were not included in the input bag. For example, zdt.with({month, hour, timeZone, calendar}) will return an object that has month and hour properties which are meaningful, _but all its other properties should be assumed to be meaningless and should not be used_.timeZone or calendar is present, and F: throw when timeZone is present or if calendar is present and both of the calendars are not ISO) and a non-throwing case G: the result will have the timeZone and/or calendar of the input if those props are present in the input.The conclusion: (A) seems like the "least bad" outcome, and there's at least one use case (albeit a kind-of-buggy one that is not a best practice) where it could reasonably be seen as useful. Specific consensus was:
withTimeZone on ZDT and keep withCalendar on ZDT and other types{} but also {calendar} or {timeZone} or {timeZone, calendar}.I discussed this offline with @justingrant, but I increasingly think that Option A is just garbage and we cannot use it in Temporal.
For example, the following code is a perfectly reasonable use case.
const jpnDateTime = Temporal.PlainDate.from("2020-05-05[u-ca=japanese]");
jpnDateTime.with("2015-01-01")
I expect that line to set my Japanese date (as fields: { era: "reiwa", year: 2, month: 5, day: 5 }) to the Julian day corresponding to 2015-01-01 in the ISO calendar. The above line is equivalent to:
jpnDateTime.with({
calendar: "iso8601",
year: 2015,
month: 1,
day: 1,
})
Based on Justin's explanation to me of Option A, the above code results in the nonsensical date 2055-01-01[u-ca=japanese]. Why?
2020-05-052015-01-012015-01-01[u-ca=japanese]. As fields: { calendar: "japanese", era: "heisei", year: 35, month: 1, day: 1 }{ calendar: "japanese", era: "reiwa", year: 35, month: 1, day: 1 }. As string: 2055-01-01[u-ca=japanese].In other words, Option A returns complete garbage for this legitimate use case.
The root cause of this problem is that it is not possible to equate the fields from one calendar into the fields of another. To illustrate: One calendar could represent dates in years, months, and days. Another could represent them in rainbows, leprechauns, and pots of gold. However, .with() should still work with those calendars as it works with any other calendar.
I listed this example earlier, but I'll reproduce it here:
// 11:11pm on Sunday, November 1, in America/Phoenix
const a = Temporal.ZonedDateTime.from("2020-11-01T23:11:00-07:00[America/Phoenix]");
// Set hour to midnight in America/Halifax
const b = a.with({
hour: 0,
timeZone: "America/Halifax",
});
Option B returns results that are guaranteed to uphold the following invariant: the return value is guaranteed to represent the requested date/time in the argument's time zone / calendar system. In the above example, you are guaranteed to get back a datetime that is 12am in America/Halifax, even if it is a different date than when you started.
Option A just returns garbage. It returns a datetime that may or may not be 12am in America/Halifax. Justin justified this by saying that it is an "approximation". I honestly don't buy that. I can't think of any use case where you would actually want a garbage approximation like this.
The only downside I've seen so far of Option B is that with() can change fields that were not requested. However, with() already does exactly this:
Temporal.PlainDate.from("2020-01-31").with({ month: 2 }); // 2020-02-29
Even though I didn't ask for the day to change, it changed nonetheless. I claim this is no different than the case when I pass a different calendar system or time zone as an argument: Temporal retains the right to adjust other fields in order to retain certain invariants.
@sffc came up with a counter-proposal in #1085 that IMHO works better than my own proposal in #1084 and any of the other proposed behaviors above. Here's what's proposed. @pipobscure: let us know what you think of this proposal below.
1. Add withPlainDate and withPlainTime methods to PlainDateTime and ZonedDateTime
PlainDate.from or PlainTime.from, respectively. No special casing or fancy cascade of types for string parsing. Example: PlainDateTime.prototype.withPlainDate = (d) => this.toPlainTime().toPlainDateTime(d)
ZonedDateTime.prototype.withPlainDate =
(d, options) => this.toPlainDateTime().withPlainDate(d).toZonedDateTime(this.timeZone, options)
from, time unit defaults are supported. So .withPlainTime({hour: 12}) will act the same as .withPlainTime('12:00').withPlainDate/withPlainTime not withDate/withTime for consistency with other methods (e.g. toPlainDate, withCalendar) where the full type name is shown in the method name. Also, withDate might encourage developers to think that it accepts a legacy Date parameter.withSomeType() pattern would also support userland ZonedTime or ZonedDate types. Simply add a new withZonedTime() or withZonedDate() method to the prototypes of PlainDateTime and ZonedDateTime.2. Rename with to withFields and throw if calendar or timeZone fields are present
with to "partial date" and/or "partial time" use cases like {hour: 12} or {day: 1}. As @sffc describes above, these use cases have misleading results when time zones or calendars are involved. Therefore, this method will throw if a non-undefined timeZone or calendar property is present in the input. This includes cases where a Temporal object is the argument to withFields because Temporal type instances will have a calendar property.with methods to withFields to make it clearer what that method is doing-- specifically to clarify that it's for individual fields and not entire dates, entire times, time zones, or calendars. For consistency we'd rename in all types, not only PlainDateTime and ZonedDateTime.withFields would unambiguously be in the calendar and time zone of the receiver, because calendar and timeZone fields are disallowed in the input.withFields would NOT accept strings, only objects, because strings have an implicit calendar and this method won't accept any calendars. This change won't break the "always accept strings and property bags wherever a Temporal object is accepted" rule because Temporal objects aren't accepted either (because they have calendars).withFields will act like with today.offset can't encounter a conflict between a new time zone and an existing offset, it's still possible for offset to be present in the input fields of ZonedDateTime's withFields, so the offset option is still needed and the default should still be preferwithPlainDate and withPlainTime will accept the same options that with does today, which is currently only overflow in PlainDateTime and overflow, disambiguation, and offset in ZonedDateTime. In the case of offset, there's no possibility of conflict with a new offset or time zone (because neither field is present in PlainDate or PlainTime), but there's still the case of wanting to maintain the same offset or to reset it, so the offset field is still useful and the default should still be prefer. 3. Solution for partial dates (including PlainMonthDay / PlainYearMonth)
with(plainMonthDay) and with(plainYearMonth) is a "partial date" use case that brings up the problematic cross-calendar behavior that Shane noted above, these types will not be accepted by withFields (because they contain a calendar field) nor will they be accepted by withPlainDate (because they don't contain all date fields). Instead, developers should use toPlainDate followed by withPlainDate. Like this:pym = Temporal.PlainYearMonth.from('2020-11');
pd = Temporal.PlainDateTime.from('2000-01-01T12:30');
pd.withPlainDate(pym.toPlainDate({ day: 1 }));
- If a MonthDay/YearMonth method is problematic (e.g. for calendar reasons) the bias is to remove it and rely on Date and docs instead. Don't bend over backwards to shoehorn functionality into these types if a less ergonomic Date+docs solution is OK.
- Don't make usability worse for other Temporal types just because a pattern doesn't work for MonthDay/YearMonth. These types are weird and making them different is OK.
Hi @pipobscure - Are you OK with the solution described in the comment above? We're coming down to the wire to get ready for Friday's workshop and we're waiting on your review. Thanks!
I really hope you three can come to agreement on this. withPlainDate and withPlainTime, and throwing on time zone or calendar in with, sound like a good way to solve the problem to me. I'm less happy about renaming with to withFields as it seems like a step backwards in API clarity and brevity, and furthermore it seems like another big last-minute rename with lots of busywork for dubious gain. The solution seems better or at least equally good if we don't rename with.
If we're not worried that developers will be surprised by with('10:00') or with(Temporal.Date.from('2020-01-01')) always throwing, then I'm OK with leaving the with name and using docs to explain why those patterns throw.
@pipobscure - Last call for feedback! We need to get this implemented ASAP.
I think the suggested solutions are even worse than just throwing!
Let’s keep things simple and not add surface. Instead just rename to withFields and throw when supplied with a calendar/timezone in god’s name.
And document that putting in temporal types is illegal (because it would throw).
Instead of withPlainDate/withPlainTime yoh can just destructure into an object and supply those values ti withFields
So plainDateTime.with('10:00') should throw?
We’ve always treated ISO strings like property bags, so no.
We’ve used Time.from to standardize this but not as that that’s used. So the string is treated just as much as it would in PlainTime.from.
What would throw is withFields(‘10:00[America/New_York]’)
PlainTime.from returns an object with a calendar property, which would cause withFields to throw. Or are you suggesting a different behavior where the withFields implementation would not directly call PlainTime.from with the string, but instead would consider a string without a calendar annotation to implicitly adopt the calendar of the receiver?
We never said we would USE .from only that we would treat ISO-Strings LIKE property bags.
For most cases using .from is a nice shortcut, but because of the throwing behaviour it’s NOT identical
We treat these strings/property-bags identical in all functions and we schemaed it for .from
But in .from the calendar can come from the property-bag or be defaulted. And the timezone just has to be there with ZonedFateTime.
If we throw in withFields when calendar/timezone is present, we should do that everywhere (so withFields in PlainDate/PlainDateTime/... should act the same way)
I'd like to clarify what you're proposing so I can be sure I understand the proposal. Assume the following code below. Which with calls do you think should throw, if any?
jpnDateTime = Temporal.PlainDateTime.from(`2020-01-01`).withCalendar('japanese');
jpnDateTime.with('10:00');
jpnDateTime.with(Temporal.Time.from('10:00'));
jpnDateTime.with(Temporal.Time.from('10:00').getFields());
Only the third one.
(If I recall correctly getFields does not return calendar/timezone; right? if it dis it would also throw)
But please RENAME to withFields
(If I recall correctly getFields does not return calendar/timezone; right? if it dis it would also throw)
getFields returns all fields, otherwise it couldn't be used for round-trip object/JSON serialization.
So if I understand you correctly, your proposal is:
with to withFieldscalendar or timeZone property is passed to withFields _(EDIT: fixed typo)_with without a calendar annotation to implicitly adopt the calendar of the receiverwithTime and withDate methods. plainDateTime.withFields({hour: 10}) would not change the value of minute in the result.Is that your proposal?
I fixed a typo in (2) above.
And in (5). ;-)
Pretty much (except for the typo in 2 where I think you meant withFields).
And if getFields contains timezone/calendar then your example line 4 would also throw.
LoL you were too quick with your typo fixes 😀
OK, then it sounds like we're all in agreement on most of the proposal. Here's the only places where I think the proposals differ:
with be renamed to withFields? @ptomato is strongly opposed, @pipobscure is strongly in favor, @justingrant is weakly in favor, @sffc is neutral.withDate and withTime method on PlainDateTime and ZonedDateTime? These methods would take a Temporal objects, string, or property bag, and would call from on the argument. Also, this withType pattern could be used for userland zoned types like withZonedTime. @sffc and @justingrant support. @ptomato weakly supports? @pipobscure: do you support?with (or, if renamed, withFields)? @sffc's proposal would throw-- only property bags would be accepted. @pipobscure's proposal would accept strings as long as no calendar or time zone annotation present, and would always interpret strings in the receiver's calendar and time zone.Can we narrow the disagreement? Anyone open to go over to the other side on any of these three questions? It feels like we're very close to a resolution, especially on the most important part which is that with/withFields should throw if a timezone or calendar is present in the input.
Here's an attempt at brokering compromise: @pipobscure - if we renamed to withFields as you proposed, but also added withDate and withTime per @sffc's proposal, and if we limited withFields to just property bags (no strings) would you be OK with that solution? If so then @ptomato would you be OK with that?
BTW, given that we all agree that with should throw for Temporal objects or property bags with (non-undefined) calendar or timezone properties, @ptomato I think this unblocks almost all of the with implementation in ZonedDateTime. String support and a new name could be layered on top depending on what we decide above. Is that helpful?
I think we should explicitly NOT have withDate and withTime! (consider this strong opposition)
I’d much prefer withFields, since the name excludes temporal objects and therefore lends clarity. But I’m not going to stress too much about it. Suffice it to say that clarity trumps brevity 100% of the time.
As to string values, I’d prefer to allow them by a country mile. But given that it would be something that’s possible to add in v2 (since it doesn’t break anything else) I’d accept leaving it out i. the interest of time.
A - Why spend extra effort, this late in the game, to rename this tried-and-true API into something more verbose and less intuitive? I disagree that it is an improvement in clarity. It seems like lots of last-minute work for dubious gain. "with() takes a property bag" is a pattern I've encountered elsewhere, but withFields() is unfamiliar and causes developers to wonder what is meant by "fields". As I said above, the solution seems better or at least equally good if we don't rename with().
B - I don't care, as I believe that passing other Temporal objects to with() is only marginally useful. Although, it was explicitly mentioned in the last plenary that the use case covered by withDate/withTime was desired, which is the _sole_ reason that I'm OK to add this API at this point. I would actually prefer it to be left for a subsequent proposal, but I accept that it's a sticking point for others.
C - I don't care about this either, since I also believe passing strings to with() is only marginally useful. But since with() already does accept strings, I'm fine with not doing the work to remove it, or I'm fine to remove it and introduce it in a subsequent proposal.
I agree on the common points, and that is indeed what I have started to implement in ZonedDateTime.
In addition I'm really disappointed to see that this is dragging on into discussions about big renames and more API surface. I don't understand why this is happening, because I thought we were all in for an attempt to go to Stage 3 in January, and this is just putting that more and more at risk.
Ok, scree the rename
OK, here's a lowest-common-denominator, future-compatible proposal to resolve the remaining open issues. Is everyone OK with this? @sffc you're the only one who hasn't weighed in today, so object now or hold your peace until V2!
withwithDate nor withTime methods. Could reconsider adding in a V2.with. Only accept property bags. Could reconsider adding in a V2.Combined with what we already agreed with earlier:
calendar or timeZone property is passed to with. This means that Temporal objects are not allowed as arguments to with because they all have calendars. plainDateTime.withFields({hour: 10}) would not change the value of minute in the result. I'm not particularly happy with this neutered result. I think it harms ergonomics with no clear reason. However, I don't have a strong objection to it.
A - I'm a 3/5 in supporting with() being renamed to withFields() if it only accepts a property bag and nothing else. But I won't block on that.
B - I'm a 4/5 in favor of withTime() and withDate(). I think it covers the ergonomics of several use cases very nicely. I haven't seen any compelling argument against them. @pipobscure says he's fairly strongly opposed, but I don't see why, other than: "Let’s keep things simple and not add surface"
C - I think we should have withTime() and withDate() that accept strings. If we really don't want those methods, though, then I guess we could accept ISO-calendar strings and copy in the result on ISO fields.
I think we're straying away from the use cases Justin listed in #1084. Let's look at how we cover the use cases with this solution:
Top Priority (optimize ergonomics for these)
3 is covered. 2 is only covered if we specifically allow strings.
1 is NOT covered. There is no viable workaround, either. If you call .getFields() on a PlainDate or PlainTime, you get an object with a calendar or timeZone property. Without any other options, users might be tempted to delete the calendar field from the return value of getFields(), which is a very risky outcome.
Next Priority (must support, but OK if ergonomics isn't perfect)
There is no nice way to cover 4. If I want to set the time of my PlainDateTime to 12:00 noon, with this proposal, I will need to write one of these monstrosities:
// Full property bag
plainDateTime.with({ hour: 12, minute: 0, second: 0, millisecond: 0, microsecond: 0, nanosecond: 0 })
// Hack with getFields()
const noon = Temporal.Time.from("12:00").getFields();
delete noon.calendar; // OUCH
plainDateTime.with(noon);
There is no solution for 5, either. Everything is manual.
Low Priority (if needed, can move developers to other APIs for these cases)
Same problems as above: you can't spread time.getFields() without deleting the calendar.
Not a Priority (these are use cases that are actively bad)
These are fine.
Just a thought exercise: since the proposal explicitly does not cover use case 1, can we invert the call site?
// Proposal from #1085
plainDateTime.withPlainDate(plainDate)
zonedDateTime.withPlainDate(plainDate)
// Inverted call site
plainDate.toPlainDateTime(plainDateTime.getTime())
plainDate.toZonedDateTime({ time: plainDateTime.getTime(), timeZone: plainDateTime.getTimeZone() });
Is there another way to write that code that's any more concise?
Caveats:
plainDateTime or zonedDateTime calendar information depending on the decision in #1087As per the meeting of 5 Nov., this isn't an issue that can be resolved to everyone's satisfaction, and we cannot delay resolving this issue any longer. The compromise that achieved consensus is the following:
with() accepts only property bags, not strings or Temporal objects, and the property bag must not have a calendar or timeZone property.withCalendar() and withTimeZone() remain as is.Other solutions were discussed, including API additions such as withPlainDate(), withPlainTime(), withFields(), as well as accepting strings in with() and accepting property bags with timeZone or calendar properties as long as they did not have any properties that were known to the receiver's calendar. The chosen compromise is future proof with respect to any of these additions, so they can be proposed for a Temporal v2 proposal.
Most helpful comment
As per the meeting of 5 Nov., this isn't an issue that can be resolved to everyone's satisfaction, and we cannot delay resolving this issue any longer. The compromise that achieved consensus is the following:
with()accepts only property bags, not strings or Temporal objects, and the property bag must not have acalendarortimeZoneproperty.withCalendar()andwithTimeZone()remain as is.Other solutions were discussed, including API additions such as
withPlainDate(),withPlainTime(),withFields(), as well as accepting strings inwith()and accepting property bags withtimeZoneorcalendarproperties as long as they did not have any properties that were known to the receiver's calendar. The chosen compromise is future proof with respect to any of these additions, so they can be proposed for a Temporal v2 proposal.