`PlainDate.from('2020-01-01T00:00Z')` is problematic

[justingrant]

Should PlainDate.from('2020-01-01T00:00Z') throw? This question came up while @ptomato was building #1749, which was based on feedback from IETF that "...Z[IANA]" strings should be valid. This issue poses the reverse question: should "Z" be prohibited for types like PlainDate?

If "Z" means "an unmoored instant that does not represent local time" then throwing in that case might make sense because the Z means that none of the date/time units are meaningful for any Plain type.

A programmer who tries to parse a Z string into a PlainDate is almost certainly committing an error. A typical example is log parsing:

getLogDateForUI(logLine) {
  // Example line: 2021-01-21T23:08:34.456 Database crash
  const timestamp = logLine.split(' ')[0];
  return Temporal.PlainDate.from(timestamp); // BUG! (if this date is assumed to match local date)
  
  // Here's what *should* be used for this case:
  return Temporal.Instant.from(timestamp).toZonedDateTimeISO(myTimeZone).toPlainDate();  
}

Another case where this could bite the developer would be parsing data that was originally in Wall+Offset format, but later changes to Instant+Z format. A naive developer might look at the original data and assume it's safe to parse into a PlainDate, but when the data format changes their app is broken but no error is shown.

Another side effect of the current behavior is that using a Z string for a relativeTo will use the UTC units, which is unlikely to match the developer's intent.

In the unusual case where the developer really wants to measure the date/time units in a Z string, there's an easy opt-in path that expresses a really clear programmer intent for all later readers of the code:

// Use the special time zone "UTC" whose local time matches Z timestamps
Instant.from('2020-01-01T00:00Z').toZonedDateTimeISO('UTC').hour

[ptomato]

That seems out of step with the decision to allow ignoring irrelevant parts of an ISO string as long as they are valid according to ISO 8601 semantics (see https://tc39.es/proposal-temporal/docs/parse-draft.html). I think throwing here would make it harder to accommodate the use case that we would otherwise have needed Temporal.parse() for. So, I don't think we should change this.

[justingrant]

I think throwing here would make it harder to accommodate the use case that we would otherwise have needed Temporal.parse() for.

What use case(s) did you have in mind for parsing an instant Z string into a PlainDate or (EDIT: or PlainMonthDay or PlainYearMonth) ? After doing a lot of investigation and prototyping in #1770, I'm now pretty curious about this.

[ptomato]

I was thinking about the use case referred to as "parsing parts" in https://tc39.es/proposal-temporal/docs/parse-draft.html. But then I remembered that use case is already no longer accommodated within Temporal because we throw even if an ignored part of the string is invalid.

I do still think that we should stick to the decision to allow ignoring irrelevant parts of an ISO string as long as they are valid. I think we went back and forth on this a lot, and while my personal preference would actually be to support "parsing parts", I think the tradeoff we made during Stage 2 was reasonable and I don't think Stage 3 is a good time to revisit that unless we have significant new information.

[justingrant]

ignoring irrelevant parts of an ISO string as long as they are valid

I think we agree on this. But we probably disagree on he meaning of "parts" and "valid". A string like
2020-12-01T00:00Z is an atomic representation of a point on the exact timeline. It has no "parts" for the same reason that Instant has no year getter. It's interchangeable with 1606780800000000000n nanoseconds or 1606780800 seconds. But it's not the same as with Temporal.PlainMonthDay.from({month: 12, day: 1}).toPlainDate({year: 2020}).toPlainDateTime() for the same reason that 0b1010 isn't interchangeable with Number("1010").toString(2) Even if their text encodings look similar, their meanings are different and shouldn't be parsed as if they're the same.

Related: default string representations of each Temporal type are supposed to be interchangeable with Temporal class instances. Developers should be able to replace an instance with the results of toString(), and vice versa, and Temporal should act the same. AFAICT, PlainXxx.from(instant.toString()) is the only place this invariant doesn't hold.

re: "valid", each Temporal type decides which subset of the ISO syntax is valid for that type. Temporal.ZonedDateTime.from(instant.toString()) throws because ZDT requires something that Instant lacks. PlainXxx types similarly require something that Instant lacks: calendar-relative date and time units, which Z strings lack for the same reason that Instant lacks year, hour, calendar, etc. Why is the string case different?

Another reason we omitted date/time getters on Instant was that interpreting UTC dates as local dates was a common problem when using legacy Date and other libraries. We wanted to avoid these errors in Temporal. Parsing a Z string with PlainDate.from(instant.toString()).month is equivalent to instant.month which we've already decided to disallow. Why is the string case different?

One reason to leave as-is would be if throwing caused a high hurdle for developers who do want to pull units out of a Z string. But the workaround is trivial: Instant.from(s).toZonedDateTime(Temporal.TimeZone.from(s)) or Instant.from(s).toZonedDateTime('UTC'). Given that "get date/time units from a Z string" is usually a developer bug, requiring extra code to opt in seems like a feature not a bug. Extra code can also flag to readers of that code that the code is doing something unusual.

If there's big benefits on the other side of the equation I'd totally be open to leaving things as-is. But I'm struggling to understand the advantage of the current state. It seems like a bug that's inconsistent with the rest of Temporal.

[ptomato]

Thanks for the summary. I think this is the main point where we disagree:

A string like 2020-12-01T00:00Z is an atomic representation of a point on the exact timeline.

My opinion: that string can mean that, and it certainly means that if it originates from Temporal, but it can also just be data from an external source, where it represents whatever the person processing the data needs it to represent. This is also the reason why we have Temporal.PlainDate.from('2020-12-01T11:52:35') discard the time portion of the string, even though it would clearly mean something more specific (a calendar date-time, not a calendar date) if it originated from Temporal and in that case the caller would be losing information. But if you have string data coming from an external source, you can choose which information to extract from it, and keeping this use case convenient is the consistency within Temporal that I personally think weighs more heavily.

[justingrant]

This is also the reason why we have Temporal.PlainDate.from('2020-12-01T11:52:35') discard the time portion of the string, even though it would clearly mean something more specific (a calendar date-time, not a calendar date)

The difference is that when decomposing 2020-12-01T21:52:35, the pieces always mean the same thing as when they were a part of the original string, because 9:52 PM on Dec 20, 2020 is still a time that's on Dec 20, 2020. Therefore, Temporal.PlainDate.from('2020-12-01T21:52:35') always means the same as Temporal.PlainDate.from('2020-12-01'). It's never a programmer bug to use the pieces separately.

Similarly, 2020-12-01T21:52:35-08:00 is also safely decomposable. The pieces always mean the same thing when they're taken apart as they did when they were part of the original string. It's not a programmer bug to use the pieces separately.

That's not true of Temporal.PlainDate.from('2020-12-01T21:52:35Z'), where using the pieces separately is very likely to be a programmer bug because 2020-12-01 has a different meaning from 2020-12-01Z aka 2020-12-01T00:00Z. The former is a calendar date, the latter is an exact timestamp that is actually November 30 in my time zone, not December 1.

The difference between wall and exact time is the most fundamental concept in Temporal. Our type structure is based on this distinction. Our API is designed to make it impossible to convert between wall<=>exact without a time zone. The case of parsing Z strings with PlainXxx.from is AFAICT the only case in Temporal where we allow calendar/wall time and exact time to be converted without a time zone. Why is this case special enough to break the pattern we worked so hard to establish elsewhere?

keeping this use case convenient is the consistency within Temporal that I personally think weighs more heavily.

Do you think that this is a common use case where ergonomics is really important? In my experience (I built log-analysis software at Splunk and Microsoft, so I've seen a lot of UTC-string parsing code over the years) extracting date parts from UTC-denominated data is relatively unusual. It's not rare, but in my experience parsing a Z string into a date is much more likely to be a bug than intentional use.

Given that the opt-in workaround (.toZonedDateTime('UTC')) is easy and that the workaround makes it much easier for future readers of the code to know what's going on, I don't understand why a relatively small ergonomics gain for an unusual use case is worth both bugs and muddying the wall vs. exact distinction Temporal tries so hard to keep distinct.

Am I missing some other common use case that justifies prioritizing ergonomics here?

[ptomato]

I think this case weighs more heavily because while Temporal has the wall time vs. exact time distinction, ISO strings do not. ISO strings have no meaning, they are either valid or invalid according to the syntactic rules of the format. Strong typing can be encouraged for strings within the context of Temporal, but IMO it can't be enforced ("stringly typed") like it can for the strongly typed Temporal objects.

After reflecting over the weekend about why I feel so strongly about something minor — I realized the above isn't actually my main concern, as it's only a medium-mildly held opinion. Rather, I am concerned that this would be a semantic change originating from within the champions group. We agreed that revisiting our decisions without external motivating feedback would be out of scope for Stage 3, and have held other champions to this agreement already. This is actually why I'm strongly against revisiting this.

[justingrant]

this would be a semantic change originating from within the champions group. We agreed that revisiting our decisions without external motivating feedback would be out of scope for Stage 3

I agree with the principle of not revisiting champions decisions. But we also fix bugs when we find them, which I assume includes not just things we decided on but implemented differently, but also things we didn't actually decide or discuss in the first place.

Any idea when we discussed and decided on the current behavior to allow PlainDate.from('2020-01-01T00:00Z')? It wouldn't be the first time I forget a discussion, but I don't remember discussing and allowing this.

[justingrant]

(BTW, this isn't urgent... fine to discuss next week!)

[pipobscure]

• Claiming that 2020-12-01T00:00Z is an atomic representation of an exact point in time is correct
• Claiming that 2020-12-01T00:00Z does not have a plain-date and plain-time part is incorrect
• All the PlainXXX types have partial information and can be combined to parse parts out of a whole valid ISO string

So I see

• PlainDate.from('2020-01-01T00:00Z')
• PlainTime.from('2020-01-01T00:00Z')
• PlainDateTime.from('2020-01-01T00:00Z')
• TimeZone.from('2020-01-01T00:00Z')
• TimeZone.from('2020-01-01T00:00+01:00[Europe/Vienna]')

all as totally valid. The Z should not be considered particularly special. It’s simply a synonym for UTC that exists mainly because it’s the universal standard rather than something special like no timezone.

Now as to throwing for invalid parts, it’s important to remember a convo we had where we were discussing: Syntactic Validity vs. Semantic Validity

We should throw on syntactic invalidity but never on semantic invalidity. And this is the very thing that makes the parsing parts use case still very valid!

I’m happy to discuss further next meeting and apologise again for having to miss today.

[justingrant]

I tried to summarize this thread below. I probably missed some things, esp. on the "don't throw" side because I'm less familiar with that one. LMK what I missed and I'll fix! Let's try to discuss either in this week's meeting or the next one.

PlainDate.from('2020-01-01T00:00Z') is OK because...

• Consistency: "We should throw on syntactic invalidity but never on semantic invalidity."
• Makes it more ergonomic to "parse into parts" and later reassemble, or other advanced parsing use cases.
• It's currently spec-ed behavior, this is a normative change, and we're in Stage 3.
• No one externally is asking to change this; changes in Stage 3 shouldn't come from the champions.

PlainDate.from('2020-01-01T00:00Z') should throw because...

• Consistency: All (other) Temporal APIs to convert exact->calendar data require a time zone.
• Consistency: All (other) Temporal APIs treat Temporal instances and strings interchangeably. Currently, PlainDate.from throws for an Instant instance but not for its default string representation.
• Quickly teaches developers that PlainDate.from is the wrong API to get a calendar date from an exact timestamp.
• If an external data source switches from DateTime+Offset format to Z format, buggy code won't fail silently.
• If we decide this is a bug, we generally fix bugs in Stage 3 regardless of who raised the issue.

It's interesting that consistency depends on what part of Temporal you're looking at. If looking at string parsing only, then "no throw" seems more consistent. If looking at Temporal overall (especially the strict distinction between exact and calendar/wall time) then "throw" seems more consistent. Is one of these consistencies more more important?

BTW, if we do decide to throw, then advanced parsing use-cases like "extract date fields from an exact-timestamp string" would use code like this:

Instant.from(s).toZonedDateTimeISO(s).toPlainDate()

This pattern may be familiar because it's also used for non-advanced use-cases like "get a date in the user's time zone".

Instant.from(s).toZonedDateTimeISO(tz).toPlainDate()

[ptomato]

The prior discussion that I had been thinking of is #428 and the document that came out of it https://github.com/tc39/proposal-temporal/pull/503/files?short_path=44e33f4#diff-44e33f44c03ab39e7308b009e6b39d2a81369bb9ffb3924a2a4e7a9775cee62c

I've put this on the agenda for October 14.

My current opinion is that I don't find most of the reasons for throwing very convincing:

• Consistency: All (other) Temporal APIs to convert exact->calendar data require a time zone.

This is true, but a string is a string, not an exact time; we don't have 'stringly-typed' APIs.

• Consistency: All (other) Temporal APIs treat Temporal instances and strings interchangeably. Currently, PlainDate.from throws for an Instant instance but not for its default string representation.

This one I am slightly sympathetic to, but I'm also skeptical that Temporal.PlainDate.from(instant.toString()) is the first thing that developers will reach for when Temporal.PlainDate.from(instant) doesn't work.

• If we decide this is a bug, we generally fix bugs in Stage 3 regardless of who raised the issue.

I don't agree that this is a bug, though, even I if did agree that we should change it. We currently have semantics for from() that are self-consistent. This discussion is about changing the semantics to something else that is also self-consistent. This is the main reason why I feel we shouldn't make this change in Stage 3.