Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider alternatives to Temporal.DateTime (ZonedAbsolute) #569

Closed
sffc opened this issue May 14, 2020 · 38 comments
Closed

Consider alternatives to Temporal.DateTime (ZonedAbsolute) #569

sffc opened this issue May 14, 2020 · 38 comments
Assignees
Labels
documentation Additions to documentation non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved

Comments

@sffc
Copy link
Collaborator

sffc commented May 14, 2020

I'm sorry to the other Temporal champions for relitigating this issue, but now that we have more evidence in the form of cookbook examples and developer feedback, I think it's at least worth revisiting the use cases behind Temporal.DateTime.

@justingrant commented in #407 (comment) that the mental model of wall clock time and wall clock arithmetic (no time zone associated with it) was unintuitive to him, a sentiment that I share and have observed when pitching Temporal to other unversed software developers.

The cookbook has some examples of Temporal.DateTime use cases. Looking through those use cases, they fall into one of a few buckets:

  1. Getting the local time of an event
  2. Converting a local time between time zones
  3. Calendar event arithmetic ("move this event to the same time next week")

Here are some ways we can keep keep most of this functionality available while making it easier to do the right thing (and harder to do the wrong thing):

  1. Remove Temporal.DateTime. Add Temporal.Absolute.prototype.getDateAndTime(tz): returns a record { date, time } containing the wall clock date and time in a certain time zone. Also allow constructing a Temporal.Absolute from a record with those two pieces.
  2. Add a time zone slot to Temporal.DateTime, making it more like Temporal.ZonedDateTime.
  3. Rename Temporal.DateTime to something more obscure and specific to its purpose.
    • Temporal.DateAndTime
    • Temporal.WallClockTime
    • Temporal.CivilDateTime
    • Temporal.LocalDateTime
  4. Do nothing (keep Temporal.DateTime as-is).

If we do nothing, the burden is on us to run an educational campaign. The question is whether we can lean on good documentation to prevent bugs that will inevitably arise from developers misunderstanding the purpose of Temporal.DateTime.

@ptomato @gibson042 @pipobscure @justingrant @ryzokuken @maggiepint

@ljharb
Copy link
Member

ljharb commented May 14, 2020

I think education will be needed regardless of which of the 4 options chosen (the 4th being, leave the spec as it is).

I don't personally find wall clock time confusing, regardless of its name, and I think it's critical that a primitive type for it exist (under whatever name).

I would prefer (modulo a bikeshed on a name) option 4, then option 3, and don't consider option 1 or 2 useful or viable.

@pipobscure
Copy link
Collaborator

For cleanliness sake I would change option 1 to:

Add absolute.getDate(tz) and absolute.getTime(tz) these double functions never sit right with me.

Which would also mean that generating an Absolute would mean always combining the tripple Date, Time and TimeZone. Which raises the question to me whether the Date even needs a calendar. If me make it creatable only via Calendar.prototype.createDate(specify what a date in this calendar is) then we could even eliminate the need for a calendar to be kept, because the created Date could be in ISO because the calendar transition was already done.

So yes, we can play with the idea, but I really don’t see any actual benefit. I will now admit to still not actually fully understanding generators (I get the concept, but that’s it). I think evidence of not fully understanding a complex scenario isn’t unexpected; especially from unversed software engineers. And it’s not actually anything to worry about.

As for option 2: it then leaves a huge whole that needs to be filled. Because the one thing that happens more than dealing with a date & time in a specific timezone, is dealing with date & time without a specific timezone. `

option 3: is basically just a name bikeshed. We’ve gone around this one at least 4 times that I can actively recall, and a 5th won’t hurt any. Though I have to admit that the likelihood of anything actually beneficial to come of it seems remote at best.

Which leaves option 4.

@pipobscure
Copy link
Collaborator

That all being said: if we fill the deficiencies with options 1 & 2 I’m happy to go there as well. It’s jus that these are rather big glaring holes that need to be filled first.

@sffc
Copy link
Collaborator Author

sffc commented May 14, 2020

For option 3, name bikeshed:

It's important for the names of the types to suggest what they are intended to be used for. We should not rely only on education.

Imagine an unversed programmer looking at Temporal for the first time. They want to represent a point in time, the most common use case. Given a choice between Temporal.Absolute and Temporal.DateTime, which are they more likely to choose?

  • Temporal.Absolute: What's an absolute? I've never seen that word in datetime contexts before.
  • Temporal.DateTime: Hmm, I recognize that term from databases, PHP, Ruby, etc.

Looking purely at the names, I have the suspicion that programmers will choose Temporal.DateTime, even if Temporal.Absolute is more generally the correct choice. (I have no data to back up this claim, other than my speculation above and observations I've made discussing with other engineers.)

I would consider the following naming scheme:

  • Temporal.Absolute becomes Temporal.Timestamp
    • People know what "timestamp" means. It sounds less scary than "absolute".
  • Temporal.DateTime becomes Temporal.DateAndTime
    • This emphasizes what the type is intended to represent, and mitigates the tendency to think of Temporal.DateTime as something that it's not.

Thoughts?

P.S. It should be noted that in both MySQL and Ruby, a "DateTime" is actually a wall clock time (no associated time zone). However, the top-voted answer in this Stack Overflow question recommends using DATETIME over TIMESTAMP, which in most cases is just wrong (it makes your database entries sensitive to the host's time zone).

@ptomato
Copy link
Collaborator

ptomato commented May 14, 2020

Option 1

I think the use case for Temporal.DateTime arithmetic, while less common than Temporal.Absolute arithmetic, is common enough that it merits having a first-class type in Temporal. There are not many instances in the cookbook, but there are not none either. I think it would be a source of bugs to ask users to roll their own even if it's relatively simple.

The use case for DateTime arithmetic is IMO the most pressing reason to keep a Temporal.DateTime type, but there are also operations that would be significantly less ergonomic to perform on a { date, time } box: comparison, balancing, and serialization to ISO string. (Though now that I write them out, that is fewer things that would become inconvenient than I thought there would be.)

Option 2

I wouldn't be opposed to adding back Temporal.ZonedDateTime (and probably renaming it Temporal.DateTime) as it would address one of the pitfalls that I found while writing cookbook examples (#240 (comment)):

ZonedDateTime was removed because it can easily be replaced by a { dateTime, timeZone } box. I'm not necessarily suggesting we reconsider that, but I did note that it's tempting to use the string returned by absolute.toString(timeZone) instead of the box. This is not necessarily correct, because even though Temporal will still deserialize the string correctly if the time zone changes its offset rules, other programs might not.

But for the same reason I mentioned in option 1 I'd want to keep Temporal.DateTime and rename it to something else. That would fill the hole that @pipobscure objected to.

Option 3

I think Temporal.Timestamp is a better name than Temporal.Absolute even regardless of this issue.

I wouldn't be opposed to Temporal.DateAndTime but I think we could come up with a better name by bikeshedding some more. If only there were a more concise name that conveyed Temporal.CalendarDateAndWallTime.

Option 4

Needs to be done anyway, regardless of this issue, IMO.

What I think would be very effective is a flowchart for choosing which type to use, in the manner of https://stackoverflow.com/a/22671607/172999

@ljharb
Copy link
Member

ljharb commented May 15, 2020

I also like Timestamp over Absolute regardless :-)

@justingrant
Copy link
Collaborator

justingrant commented May 15, 2020

I agree with @ljharb that the "wall clock time" concept of DateTime is NOT confusing. It took me a while to understand that (unlike similarly-named concepts in other APIs) Temporal.DateTime had no linkage to time zones nor real-world instants. But once I understood that, its meaning was clear to me and I think would be clear to anyone. Overall the current design of Temporal seems like a reasonable "assembly language" of low-level primitives that can be used together to perform any date/time related use-case.

I also agree with @sffc's and others' concerns about the usefulness of DateTime for many important use-cases.

I also have three serious and inter-related concerns with the current design of DateTime:

Below are details about each of these concerns. Below that is a rollup of some concrete suggestions that I think might address these concerns. FWIW, @ptomato's Option #2 would work best for the use-cases I'm familiar with.

DST Safety

Almost all (all?) non-trivial DateTime operations are unsafe for DST, where "unsafe" means "easy for developers to write code that delivers expected results (and passes unit tests!) almost all the time, but delivers unexpected results around DST transitions".

These DST-unsafe operations include:

  • arithmetic: plus, minus, and difference
  • compare, which is based on subtraction
  • with and from if hour > 24, or if largestUnit is hours (or smaller) and the Date portion is nonzero

To understand why these operations are unsafe, here's some use-cases with sample code.

#407 (comment) has a few possible options to fix DST safety:

  1. Add timeZone to DateTime, or derive a new timezone-aware class with similar API to DateTime
  2. Add a required timeZone parameter to all DST-affected methods. Users who want to ignore DST issues can pass UTC or a special "zoneless" timezone. (see below)
  3. Move all DST-afffected operations to the TimeZone class and remove them from DateTime.
  4. Remove DST-afffected operations from DateTime, which would IMHO severely limit the usefulness of Temporal.

DX/Verbosity for common use-cases

Once DST safety is the default, my next concern is the cognitive load and verbosity required to write DST-safe code. The biggest challenges:

  • lots of round-tripping required between Absolute and DateTime
  • cognitive load of figuring out which operations need to be performed in Absolute vs. DateTime context
  • Having to pass the same timezone into so many subsequent APIs when I already told Temporal what timezone I wanted!

Here's an illustrative use-case: My calendar just showed me a reminder. At this same (local) time tomorrow my flight will depart for London. My flight is 8 hours long. What local time will it be when I get there?

// correct logic is hard
function getArrivalTimeFromReminder(reminderInstant, departureTz, arrivalTz) {
  const localNow = reminderInstant.inTimeZone(departureTz)

  // Do date math in local timezone in case DST starts/ends tonight 
  const departureLocal = localNow.plus({days:1})
  const departureInstant = departureLocal.inTimeZone(departureTz)

  // Do hours math in Absolute because the length of the flight is 
  // not affected by DST
  const arrivalInstant = departureInstant.plus({hours:8})
  const arrivalLocal = arrivalInstant.inTimeZone(arrivalTz)
  return arrivalLocal.getTime()
}

// Here's what an easier API could look like
function getArrivalTimeFromReminder(reminderInstant, departureTz, arrivalTz) {
  // `localNow` retains the timezone used when it was created
  const localNow = reminderInstant.inTimeZone(departureTz)

  // `days` is incremented in clock time, while `hours` is incremented in UTC
  const arrivalLocal = localNow.plus({days:1, hours:8}).inTimeZone(arrivalTz)
  return arrivalLocal.getTime()
}

Here's the convention that makes an easier API possible:

  • DST-safe code usually needs to perform Time arithmetic in Absolute terms (e.g. "meet me at the bar in 2 hours, not 1 or 3 hours if DST happens tonight")
  • Date arithmetic should use DateTime/Wall-Clock semantics, e.g. "Alexa, reschedule my Friday night dinner reservation for one week later" where local time stays constant as days/weeks/etc are changed.

"DateTime" name vs. developer expectations

A common pattern in API design is a "main" vs. "expert-only" API:

  • The "main" API targets mainstream use-cases, is optimized for developer experience, and has defaults that discourage usage patterns that are likely to cause bugs.
  • The "expert-only" UI is more comprehensive and/or better performing, but also harder to learn and/or more verbose and/or easier to cause bugs and/or lacks DX niceties.

An example of doing this badly is in the C standard library: strcpy(dest, src) copies two strings, and so does strncpy(dest, src, length). The former API is shorter and takes fewer params so seems like it should be the "main" API, but in fact strncpy is the actual main API because strcpy is so vulnerable to buffer overflows. API naming nudges novices to use the "wrong" API, which is bad.

An example of doing this right is Map vs. WeakMap. Their API surface is nearly identical, but novices will gravitate to Map because the name is shorter and clearer. Another minor advantage is that Map is earlier in the alphabet so will show up first in autocomplete and written docs. Finally, what novice wants to write code that's "weak"? ;-)

Luckily, most Temporal types like Temporal.Date are useful both as "main" and "expert" APIs. The exception is Temporal.DateTime, which is IMHO an "expert" API because it's hard to use for real-world, DST-safe use-cases. (which might get even harder depending on what DST-safety changes are decided)

If Temporal retains a zoneless date+time class, then I'd suggest a longer and less alluring name (e.g. ZonelessDateTime?) to guide novices to the "main" API.

Wishlist

Here's a wishlist of suggestions to address the concerns above. Y'all understand this space more than I do, so feel free to toss ideas you think won't work.

  • Temporal should offer a "main" date+time API that knows its timezone so it can provide better DX for DST-safe operations on local times. I think DateTime would be a fine name for this API. It'd mostly match the existing DateTime API with some differences:
    • Timezone should be stored when the object is constructed.
      • Instances constructed via methods (e.g. plus, minus, etc) should retain the time zone.
      • The timeZone parameter should be removed from all methods.
      • Timezone MUST be a required parameter to the constructor
      • To get zoneless behavior, should we have a special "zoneless" TimeZone? This would match UTC behavior but might help distinguish user intent vs. UTC.
      • ISO zoneless strings can be supported by from, but should require some sort of opt-in option to from.
    • This type should also store its Absolute timestamp, so that it can tell the difference between the 1:00AM before DST ends and 1:00AM when DST ends. And for comparisons during the hour before DST ends.
      • compare should only use the timestamp, never the clock time.
    • Property getters and getFields should expose the time zone and timestamp
    • Operations that change the time (e.g. plus({hours: 2})) should should first convert to UTC, then perform the operation, then convert back to local time in the same time zone. Operations that change the date (e.g. {weeks: 1}) should just change the date without changing the local time.
    • Absolute.inTimezone() should return an instance of this type, not the zoneless type.
    • This type should have an hoursInDay property or method to help apps detect DST quirks.
  • Temporal should also have an "expert" API for zoneless date+time values. One option would be to simply give today's DateTime a longer name (e.g. ZonelessDateTime). Another option would be a special timezone value, e.g. TimeZone.Zoneless or TimeZone.Null.
  • Consider restricting Absolute operations to largestUnit: hours to encourage use of the local-time classes (or Date) for operations that might be affected by DST
  • I agree that Timestamp is a clearer name than Absolute. "Timestamp" also matches what a few other platforms call their similar concept.

As a newbie to this proposal and with my app developer's perspective, I'm admittedly ignorant of many of the requirements and most of the history. So I apologize in advance if I'm re-hashing settled arguments, missing important use-cases, or presenting naive suggestions. Overall I'm really impressed with the work that y'all have done!

@kaizhu256
Copy link
Contributor

as product-developer, the following naming-convention seems intuitive (to me) with minimal education:

  • Temporal.DateTimeLocal - for [client-side] ui (use client's system-default zone and dst).
  • Temporal.DateTimeZoned - for [client-side] ui (use user-specified zone and dst)
  • Temporal.DateTimeUTC - for [server-side] database-interactions

though not strictly necessary, Temporal.DateTimeLocal is convenience-stub for Temporal.DateTimeZoned, in common-case where product-developers are too-lazy to explicitly supply zone-offsets and dst.

@pipobscure

This comment has been minimized.

@kaizhu256

This comment has been minimized.

@sffc
Copy link
Collaborator Author

sffc commented May 15, 2020

TimeZone slot in DateTime

The question about whether to include a TimeZone slot in DateTime is related to the question of whether to include a Calendar slot. It's also worth pointing out that Temporal.Absolute does not have or need either TimeZone or Calendar.

Food for thought: Why should DateTime have a Calendar slot but not a TimeZone slot?

My answer to this question: The concept of a wall clock time is calendar-agnostic, but it has no useful manifestation without the existence of a calendar system. In order to get the year, month, or day, or to perform arithmetic, you need a calendar system. However, the removal of a time zone is what defines a wall clock time. Therefore, it makes sense for a wall clock type to have a calendar slot but not a time zone slot.

I had originally proposed in #268 that Date and DateTime not have a Calendar slot, and that a Calendar should be provided when performing any calendar-sensitive operations. However, I was later convinced that we should indeed have a Calendar slot, in large part because of developer expectations that they should give the calendar once, and have it used for all calculations from that point forward. This is essentially the same line of reasoning as this bullet from @justingrant's post: "Having to pass the same timezone into so many subsequent APIs when I already told Temporal what timezone I wanted!"

Flight to London

Here's an illustrative use-case: My calendar just showed me a reminder. At this same (local) time tomorrow my flight will depart for London. My flight is 8 hours long. What local time will it be when I get there?

I think this is a beautiful example of the Temporal data model working as designed. You need to add two quantities: 1 day in wall clock time, and 8 hours in absolute time. You therefore perform one calculation with Temporal.DateTime, and the other calculation with Temporal.Absolute.

The existing Temporal data model is the result of a lot of design, and I think the distinction between wall clock time and absolute time is a good one.

ZonedAbsolute

  • This type should also store its Absolute timestamp, so that it can tell the difference between the 1:00AM before DST ends and 1:00AM when DST ends. And for comparisons during the hour before DST ends.

The type you are proposing is essentially a ZonedAbsolute. That's the same data model as legacy JavaScript Date, the thing we know is broken and that we are trying to replace.

However, you bring up a good point about a potential fatal flaw in the ZonedDateTime data model, which is the inability to distinguish between overlapping times at a DST transition.

Suppose for the sake of argument that we were to adopt your data model. (For the purposes of this post, I will use the term ZonedAbsolute, but we could change the name later or replace DateTime with this new data model.) The slots for this new type would be:

  1. [[EpochNanoseconds]]
  2. [[TimeZone]]
  3. [[Calendar]]

We would debate whether to make arithmetic have the semantics @justingrant proposed (days and higher are wall clock time, hours are lower are absolute time), or to keep arithmetic all in wall clock time.

The method to convert from Absolute to ZonedAbsolute would accept both TimeZone and Calendar. We would debate about whether those arguments are optional (default UTC/ISO) or required.

ZonedAbsolute.prototype.toLocaleString would use the time zone and calendar encoded in the data model. Absolute.prototype.toLocaleString would use the environment's time zone and calendar.

Other than that, the semantics of ZonedAbsolute would be indistinguishable from DateTime.

How would ZonedAbsolute affect the rest of the cookbook examples?

@justingrant
Copy link
Collaborator

Thanks @sffc for the clear and even-handed summary. The app-dev use cases that I'm most familiar with would benefit a lot from something like ZonedAbsolute. Specifically, it'd make it much easier for developers to write DST-safe code.

To clarify, my concern about DateTime isn't that the wall-clock time concept is too hard for devs to learn. Rather, it's that best practices for DST safety are too hard for most devs to learn and apply. In my experience, without a higher level of abstraction like ZonedAbsolute that makes DST best practices the default behavior, most developers will write naive code that breaks twice per year.

How would ZonedAbsolute affect the rest of the cookbook examples?

Here's four tiny ZonedAbsolute use-cases. Each has three variations:

  • what developers would probably write if they don't know about DST pitfalls (in my experience this is at least 90% of devs)
  • DST-safe code using ZonedAbsolute. This is usually identical to the naive code, but is DST-safe.
  • DST-safe code with the current API
// Get clock time from a {Temporal.Date, Temporal.Time}
// e.g. from date/time picker components
//
// Problem: novices won't know they need to validate/coerce
// the time in case it's an invalid time like 2:30AM on a day 
// where DST starts at 2:00AM.  BTW, I checked how MS Outlook
// handles this, and it will silently coerce invalid times 
// to valid times. No user-facing error.

// naive 
function getCurrentPickerTime(datePicker, timePicker) {
  return DateTime.from(DatePicker.value)
    .with(TimePicker.value)
} 

// ZonedAbsolute (same as naive)
function getCurrentPickerTime(datePicker, timePicker) {
  return DateTime.from(DatePicker.value)
    .with(TimePicker.value)
} 

// DST-safe (current API)
function getCurrentPickerTime(datePicker, timePicker) {
  return DateTime.from(DatePicker.value)
    .with(TimePicker.value)
    .Temporal.Absolute.inTimeZone(Temporal.now.timeZone())
    .inTimeZone(Temporal.now.timeZone())
} 

// Move a deadline one day later.
//
// Problem: novices won't know they need to switch into the
// local time zone to account for possible DST

// naive
function oneDayLater(absolute) {
  return absolute.plus({day: 1})
}

// ZonedAbsolute (same as naive)
function oneDayLater(zonedAbsolute) {
  return zonedAbsolute.plus({day: 1})
}

// DST-safe (current API)
function oneDayLater(absolute) {
  return absolute.inTimeZone(Temporal.now.timeZone())
    .plus({day: 1})
    .inTimeZone(Temporal.now.timeZone())
}

// Postpone a reminder by one hour.
//
// Problem: DST transitions should be ignored

// naive - "Yesterday when I was writing `oneDayLater`,
// my manager yelled at me for writing DST-unsafe code,
// so to avoid the same fate I'll just copy/paste the 
// DST-safe code here and change the units."
function oneHourLater(absolute) {
  return absolute.inTimeZone(Temporal.now.timeZone())
    .plus({hour: 1})
    .inTimeZone(Temporal.now.timeZone())
}

// ZonedAbsolute (not same as this particular
// copypasta-based naive example. but would be what
// an actual naive developer would write)
function oneHourLater(zonedAbsolute) {
  return zonedAbsolute.plus({hour: 1})
}

// DST-safe (current API)
function oneHourLater(absolute) {
  return absolute.plus({hour: 1})
}

// Return a sorted list of local Date & Time of flight arrivals
// in local time zone
//
// Problem: For one hour before DST ends,
// real-world sort order won't match clock-time sort.

// naive - "To avoid creating two copies of the
// array I'll get local times and then sort them."
function getSortedArrivals(arrivals) {
  return arrivals.map(a => a.inTimeZone(Temporal.now.timeZone()))
    .sort(Temporal.DateTime.compare)
}

// ZonedAbsolute (same as naive except class name)
function getSortedArrivals(arrivals) {
  return arrivals.map(a => a.inTimeZone(Temporal.now.timeZone()))
    .sort(Temporal.ZonedAbsolute.compare)
}

// DST-safe (current API)
function getSortedArrivals(arrivals) {
  return [...arrivals].sort(Temporal.Absolute.compare)
    .map(a => a.inTimeZone(Temporal.now.timeZone()))
}

@justingrant
Copy link
Collaborator

justingrant commented May 16, 2020

Here's another use case that I forgot to include above: string formatting including a time zone, like legacy Date's default localized string representation in console.log. This use case is unrelated to DST but ZonedAbsolute would make it easier.

// Format a date, time, timezone string
//

// ZonedAbsolute
function displayTodayNoon() {
  return Temporal.now
    .zonedAbsolute()
    .with(new Temporal.Time(12))
    .toLocaleString()
}

// Current API. Requires learning about Intl display options
// to use the default display format for a timezoned date/time.
// Also requires discovering that the correct format method
// lives on Absolute which is non-obvious because nothing else
// on that class relates to local clock time.
function displayTodayNoon() {
  return Temporal.now
    .dateTime()
    .with(new Temporal.Time(12))
    .inTimeZone(Temporal.now.timeZone())
    .toLocaleString(undefined, {timeZone: Temporal.now.timeZone().name})  
}

@sffc
Copy link
Collaborator Author

sffc commented May 19, 2020

Here's another potential model using ZonedAbsolute, which also relates to the open question of the default calendar (#292).

If we add ZonedAbsolute, it can be an intermediate type between Absolute and DateTime. It would have a time zone, with the capability to change time zones, but no calendar or calendar-specific accessors like year/month/day. Arithmetic would all be performed as absolute time, not wall clock time. If you need calendar-dependent operations (accessors and wall clock arithmetic), you call .withCalendar() and do what you need to do in DateTime space.

Note: This model of ZonedAbsolute may make Absolute obsolete.

I have more details and a nice graphic shown here:

https://tc39.es/proposal-temporal/docs/calendar-draft.html#new-non-calendar-types-option-5

@sffc sffc changed the title Consider alternatives to Temporal.DateTime Consider alternatives to Temporal.DateTime (ZonedAbsolute) May 20, 2020
@ptomato ptomato added this to the Stage 3 milestone May 28, 2020
@ptomato
Copy link
Collaborator

ptomato commented May 28, 2020

We talked about this in passing in the May 28 meeting. The initial polyfill is not going to ship with a ZonedAbsolute, but we would like to revisit it for Stage 3. @justingrant is working on a proof of concept. @gibson042 indicated he would prefer ZonedDateTime and we intend to discuss the pros and cons on this thread.

@justingrant
Copy link
Collaborator

I just published #700 which is a draft PR of a "ZonedAbsolute" proof-of-concept. It differs from @sffc's proposal in a few ways so I gave it a different placeholder name ("LocalDateTime") to avoid ambiguity while we discuss it. Feedback appreciated. Apologies for the length of the writeup!

Ms2ger pushed a commit that referenced this issue Nov 6, 2020
Ms2ger pushed a commit that referenced this issue Nov 6, 2020
This adapts all the relevant cookbook examples (except the meeting planner
which is blocked on a bug in ZonedDateTime.toLocaleString) to use
ZonedDateTime. Most of these were identified in Justin's original pull
request.

Co-authored-by: Justin Grant <justingrant@users.noreply.github.com>

See: #569
ptomato added a commit that referenced this issue Nov 6, 2020
The ZonedDateTime's time zone wins over the system's default time zone.
If you explicitly give a time zone in the formatter options, and you try
to format a ZonedDateTime with a time zone which is different from the
explicitly given time zone, then toLocaleString() will throw an exception.

formatRange() and formatRangeToParts() require ZonedDateTimes with the
same time zone.

See: #569
ptomato added a commit that referenced this issue Nov 6, 2020
Now that ZonedDateTime.toLocaleString() works, we can fix this example. It
becomes much simpler using ZonedDateTime.

See: #569
ptomato added a commit that referenced this issue Nov 7, 2020
This operation will be reused in ZonedDateTime.until() and since().

See: #569
ptomato added a commit that referenced this issue Nov 7, 2020
The rounding in ZonedDateTime.toString() only affects time units. Unlike
the rounding in ZonedDateTime.round(), it should be exact time rounding,
so that the actual difference between the raw and rounded exact times is
not affected by time zone offset shifts.

This is the same outcome as in round() (and we add a test for both round()
and toString() to prove it) but in toString() it can be implemented with
fewer observable calls since we don't need to be able to round to units
that are affected by DST shifts.

See: #569
Ms2ger pushed a commit that referenced this issue Nov 9, 2020
The rounding in ZonedDateTime.toString() only affects time units. Unlike
the rounding in ZonedDateTime.round(), it should be exact time rounding,
so that the actual difference between the raw and rounded exact times is
not affected by time zone offset shifts.

This is the same outcome as in round() (and we add a test for both round()
and toString() to prove it) but in toString() it can be implemented with
fewer observable calls since we don't need to be able to round to units
that are affected by DST shifts.

See: #569
ptomato added a commit that referenced this issue Nov 9, 2020
And make a small adjustment to the polyfill to match.

See: #569
ptomato added a commit that referenced this issue Nov 9, 2020
The ZonedDateTime's time zone wins over the system's default time zone.
If you explicitly give a time zone in the formatter options, and you try
to format a ZonedDateTime with a time zone which is different from the
explicitly given time zone, then toLocaleString() will throw an exception.

formatRange() and formatRangeToParts() require ZonedDateTimes with the
same time zone.

See: #569
ptomato added a commit that referenced this issue Nov 9, 2020
Now that ZonedDateTime.toLocaleString() works, we can fix this example. It
becomes much simpler using ZonedDateTime.

See: #569
ptomato added a commit that referenced this issue Nov 9, 2020
This operation will be reused in ZonedDateTime.until() and since().

See: #569
Ms2ger pushed a commit that referenced this issue Nov 10, 2020
This operation will be reused in ZonedDateTime.until() and since().

See: #569
Ms2ger pushed a commit that referenced this issue Nov 10, 2020
The ZonedDateTime's time zone wins over the system's default time zone.
If you explicitly give a time zone in the formatter options, and you try
to format a ZonedDateTime with a time zone which is different from the
explicitly given time zone, then toLocaleString() will throw an exception.

formatRange() and formatRangeToParts() require ZonedDateTimes with the
same time zone.

See: #569
Ms2ger pushed a commit that referenced this issue Nov 10, 2020
Now that ZonedDateTime.toLocaleString() works, we can fix this example. It
becomes much simpler using ZonedDateTime.

See: #569
Ms2ger pushed a commit that referenced this issue Nov 10, 2020
And make a small adjustment to the polyfill to match.

See: #569
@ptomato
Copy link
Collaborator

ptomato commented Nov 10, 2020

Opened #1171 for the last item on the checklist since it is the same as the last item on #856's checklist as well. Closing this now, everything else has landed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Additions to documentation non-prod-polyfill THIS POLYFILL IS NOT FOR PRODUCTION USE! spec-text Specification text involved
Projects
None yet
8 participants