icu4x icon indicating copy to clipboard operation
icu4x copied to clipboard

Should DateTimeFormatter be renamed to PlainDateTimeFormatter?

Open sffc opened this issue 3 years ago • 10 comments

The ECMAScript Temporal champions agreed long ago that a type named DateTime is fraught. The reasons for this are fairly well documented in the Temporal docs as well as in the Temporal repository. The end result is that Temporal has the following types:

  • Temporal.Absolute (under the hood: nanoseconds since Unix epoch)
  • Temporal.ZonedDateTime (under the hood: time zone, calendar, and nanoseconds since Unix epoch)
  • Temporal.PlainDateTime (under the hood: calendar and ISO datetime)

Should we learn from Temporal's experience and rename DateTimeFormatter to PlainDateTimeFormatter since this most closely reflects what it does?

An early win from this change is that people will be less likely to reach for DateTimeFormatter thinking that it's a swiss army knife that it's not, since it doesn't support time zones (and therefore not time lengths long or full).

@zbraniecki @Manishearth @nordzilla

sffc avatar Sep 20 '22 00:09 sffc

Is there a paper trail on this decision? My weakly held initial position is that it is a mistake and DateTime vs ZonedDateTime is analogous to Time vs DateTime. We wouldn't try to name it PlainTime vs DateTime, and it doesn't scale to PlainPlainTime either :)

zbraniecki avatar Sep 20 '22 00:09 zbraniecki

As stated in #2577 I do not believe in usage of API name to educate developers. Therefore making a core type name longer to make people aware that this is non-zoned is imho a bad use of API and a bad DX.

zbraniecki avatar Sep 20 '22 00:09 zbraniecki

I asked @ptomato for the paper trail.

My question is more about whether we should use Plain as a value prop of being consistent with Temporal.

sffc avatar Sep 20 '22 01:09 sffc

I don't have a strong opinion here, but I have been bitten by the lack of all timelengths in the past.

Manishearth avatar Sep 20 '22 03:09 Manishearth

Philip will follow up with more details, but I can share a summary of why PlainDateTime and ZonedDateTime ended up being named that.

Note that I have no opinion about what things should be called in ICU4X. I can just share the reasoning for Temporal. YMMV.

First, some background. There are three groups of use cases for a date/time API that knows about clock time and calendar date:

  1. Use cases that require knowing the time zone, e.g. "Add one day" or "How many hours has it been since yesterday at 9:00?"
  2. Use cases that work equally well whether or not you know the time zone, e.g. "What hour is it?" or "Is it a leap year?"
  3. Use cases that require ignoring the time zone, e.g. in a day-planner UI, the visual height of a meeting may be the same even if DST skips or repeats an hour.

(1) and (2) are very common in real-world software, while cases of (3) are rare enough to be listed in a few bullet points.

So a "zoned" type is safe to use in almost every use case, while a zoneless type is not. Furthermore, the bugs produced by a zoneless type tend to be intermittent because they only show up during some hours on a few days per year, and even then only in some countries. Intermittent bugs are evil and should be avoided.

FWIW, I've seen this evil up close when I worked at a startup that spent years of engineering work undoing a disastrous mistake made by the 22-year-old founders to use Microsoft SQL Server's zoneless datetime2 type.

Anyway, my preference would have been to do what Rust did, where there's a "safe" type with a short, friendly name like DateTime and an "unsafe" type with a scary name like NaiveDateTime.

This didn't get consensus, so we chose another solution: put the zoned and zoneless type on equal footing because both have a prefix. I don't think this is an ideal outcome, but it's vastly better than the alternative of naming the DST-unsafe, bug-prone zoneless type DateTime. That naming would have encouraged millions of inexperienced JS developers to assume that it's the right type to use for their use case because the other type (ZonedDateTime) is longer and scarier-sounding. At least now, developers are forced to think about which is the right type to use.

A good side-effect of using a prefix for all non-zoned types is that it avoids ambiguity between PlainDate and the legacy Date type.

I hope this history is helpful. I'm happy to answer more questions if needed.

justingrant avatar Sep 20 '22 05:09 justingrant

More paper trail: https://github.com/tc39/proposal-temporal/issues/11 followed by https://github.com/tc39/proposal-temporal/issues/33 followed by https://github.com/tc39/proposal-temporal/issues/707

sffc avatar Sep 20 '22 14:09 sffc

This didn't get consensus

That's what I'm curious about, because your preference is also my preference.

The solution chosen feels to me like a decent fallback, but I do not understand why would we have to go for it, when the top choice is optimal and available.

A good side-effect of using a prefix for all non-zoned types is that it avoids ambiguity between PlainDate and the legacy Date type.

Right, which can be seen as a value prop for JS, but not for us and I'd count it as a pretty minor one in JS case compared to the accumulated cost of the unnecessarily-elongated-name over time. 10 years from now, I hope, the old Date will be deprecated and all good linters will warn/reject it, while for another 30 years people will have to write Plain just because there was a transition period and lack of consensus on scaling naming with detail.

zbraniecki avatar Sep 21 '22 20:09 zbraniecki

I do not understand why would we have to go for it, when the top choice is optimal and available.

The explanation is straightforward: not everyone in our champions group shared my (and your!) opinion that favoring the zoned type was the right way to go. Smart people with different experiences and temperaments will approach problems differently, and sometimes what seems like a top choice to one person seems like a bad idea to another.

Our group had a diversity of opinions ranging from "the zoneless type is a dangerous footgun and should be explicitly discouraged" to "the zoneless type is great and anyone who can't use it properly really shouldn't be writing software". After months of arguing and several polls of the wider developer community, in the end we agreed to meet in the middle on the name.

My priority then was avoiding the disaster of naming the zoneless type DateTime. I was willing to trade extra typing for PlainDate and PlainTime in return for avoiding decades of DST bugs caused by naive developers picking the wrong type. I still think this was the right tradeoff, even though I wish that we didn't have to make it.

BTW, there were also valid arguments for why naming it ZonedDateTime was good:

  • It aligned with Java's type of the same name, which made sense because Java's type was very similar in spirit and behavior to Temporal's .
  • It avoided users coming from other platforms that had a type named "DateTime" from assuming that JS's type would behave identically.
  • It clearly highlighted its superpower (handling time zones) which might help with discoverability in docs, in IDE autocomplete, etc.

for another 30 years people will have to write Plain just because there was a transition period and lack of consensus on scaling naming with detail.

It's a JavaScript tradition. As my old friend === said, why use fewer characters when more is better? 😄

justingrant avatar Sep 22 '22 07:09 justingrant

not everyone in our champions group shared my (and your!) opinion that favoring the zoned type was the right way to go.

That is given, but I'm wondering what arguments were listed. It may be that the arguments listed by the opponents were exactly the ones I disagree with - in which case my disagreement likely holds - or maybe there were arguments I have not thought about - in which case my disagreement can be severely weakened. Hence I asked for paper trail back link.

After months of arguing and several polls of the wider developer community, in the end we agreed to meet in the middle on the name.

Meeting in the middle is another design choice that I think more often than not leads to suboptimal solutions.

zbraniecki avatar Sep 22 '22 07:09 zbraniecki

My priority then was avoiding the disaster of naming the zoneless type DateTime.

This is my main potential worry with DateTimeFormatter; it looks like thing thing people should use by default, but it doesn't support time zones.

sffc avatar Sep 22 '22 16:09 sffc

My priority then was avoiding the disaster of naming the zoneless type DateTime. I was willing to trade extra typing for PlainDate and PlainTime in return for avoiding decades of DST bugs caused by naive developers picking the wrong type. I still think this was the right tradeoff, even though I wish that we didn't have to make it.

So ICU4X has a bit of a different set of tradeoffs, because we don't mandate a single date type, we allow people to pass in anything that implements the right trait.

We do not prima facie couple DateTime and TimeZone types: our ZonedDTF requires you to pass in a DateTime and a TimeZone, but due to the abstraction they can be two references to the same value.

We do ship with a Date, Time, DateTime (Date + Time), and CustomTimeZone type, but like I said they are not mandatory to use and you can use your own.

Due to all this I think people will not have the same mental association with Plain, so I'm not sure that naming will achieve the same results.

Manishearth avatar Sep 22 '22 18:09 Manishearth

  • @echeran - jodatime is an arithmetic library. In that sense, an instant / zoned datetime makes sense. But for ICU4X, we're a formatting library, and we allow you to combine together different parts. Not using a Plain prefix is internally consistent.
  • @sffc - We would more likely also change the names of Date to PlainDate, etc. But I abstain from that decision.

Consensus of the ICU4X team: keep DateTimeFormatter. @zbraniecki @echeran @Manishearth

sffc avatar Sep 22 '22 18:09 sffc