size-suggestion: Consider merging getNextTransition/getPreviousTransition into one with a direction option

[justingrant]

Should we merge getPreviousTransition and getNextTransition into a single method like this?

Temporal.TimeZone.prototype.getTransition ( 
  _startingPoint_: Temporal.Instant,
  _direction_: 'previous' | 'next' 
): Temporal.Instant | null

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

This would be especially worth considering if we remove the Time Zone class (see #2826) and have to move this functionality into the ZonedDateTime type, so we'd end up with something like this:

Temporal.ZonedDateTime.prototype.getTimeZoneTransition ( 
  _direction_: 'previous' | 'next' 
): Temporal.ZonedDateTime | null

[gibson042]

This would be a big ergonomic loss IMO. getNextTransition(start) and getPreviousTransition(start) are much more clear than getTransition(start, direction), especially at development time when the expected data type and semantics of direction are not necessarily at hand.

[ptomato]

I'm neutral on this one. The methods would certainly become more annoying to use, but I don't expect they are used very often.

[FrankYFTang]

My origional proposal at https://docs.google.com/document/d/16Rd43XB8z6iLC2c9AfF--unDFg_t476tI1PFe7PbbcM/edit#heading=h.xs8xipj8fzpn

getNextTransition(start) and getPreviousTransition(start) are much more clear than getTransition(start, direction), especially at development time when the expected data type and semantics of direction are not necessarily at hand.

For those developers who "at development time when the expected data type and semantics of direction are not necessarily at hand", how would they decide how to write their code to call getNextTransition or getPreviousTransition ?

[gibson042]

For those developers who "at development time when the expected data type and semantics of direction are not necessarily at hand", how would they decide how to write their code to call getNextTransition or getPreviousTransition ?

The scenario is that they know that they want e.g. the next transition, but not how to specify that for a combined method (e.g., getTransition(start, 1) vs. getTransition(start, "next") vs. getTransition(start, "forward") vs. getTransition(start, { direction: "forward" }) vs. …).

[FrankYFTang]

but not how to specify that for a combined method

hum? why? They can read MDN the same way as they read MDN to learn about how to call getNextTransition and getPreviousTransition right?

[gibson042]

Sometimes, but sometimes not. And regardless, we don't want APIs to be so opaque that such lookups are necessary—once someone learns about a function, further confusion regarding how to use it (and often more importantly, how to read it) should be minimized.

This is the same reason why we prefer property bags to long parameter lists, why "SmooshGate" was such a big deal, why we have String {pad,trim}{Start,End} and {starts,ends}With methods, why reduce was not the final Array method (and why people complain about splice), why DataView has distinct methods for BigInt64, Float{32,64}, {Int,Uint}{8,16,32}, etc.

[FrankYFTang]

@gibson042 be aware, when I draft Ideas to Reduce Temporal API Surface without Reducing Functionality, it is written as a counterproposal to issues/2826 which remove all TimeZone protocol, including getNextTransition/getPreviousTransition . I think it is much ergonomic to support one function getTransition than none of them. No code could be discovered if they got removed from the Temporal proposal. so the choices facing us is

1. Keep the status quo to have both getNextTransition/getPreviousTransition
2. Remove getNextTransition/getPreviousTransition with all TimeZone
3. Merge getNextTransition/getPreviousTransition into one getTransition to reduce the need for size-suggestion: Remove Temporal.TimeZone class and protocol #2853

[gibson042]

@FrankYFTang see #2853 (emphasis mine):

Add ZonedDateTime.p.getTimeZoneTransition('next' | 'previous') (or a pair of methods instead)

[littledan]

I don't think my point of view is very important here because this is a rare expert feature, so I don't think we need to care a gigantic amount about ergonomics. But it feels slightly wrong to merge two functions and then control which one you're accessing with a flag--I wouldn't normally want to design an API like this. This change seems like overfitting to the current state of V8.

[FrankYFTang]

to merge two functions

but... why this need to be one function?
if you compare
https://tc39.es/proposal-temporal/#sec-temporal.timezone.prototype.getnexttransition
and
https://tc39.es/proposal-temporal/#sec-temporal.timezone.prototype.getprevioustransition

The only differences between then is one call
GetNamedTimeZoneNextTransition and the other call
GetNamedTimeZonePreviousTransition
and there no other differences.

And then if you read
https://tc39.es/proposal-temporal/#sec-temporal-getnamedtimezonenexttransition
and
https://tc39.es/proposal-temporal/#sec-temporal-getnamedtimezoneprevioustransition

the only differences between them is one states

"the first time zone transition after epochNanoseconds"
and the other states
"the last time zone transition before epochNanoseconds"

and there are no other differences beyond that.

[justingrant]

Meeting 2024-05-23: At the upcoming June 2024 TC39 plenary meeting, we'll propose removing the TimeZone type (see #2853). So getNextTransition/getPreviousTransition needs to change regardless. Given that these methods are relatively-advanced use cases and that we have to make a change anyway, we'll shave one method by combining them ino a single method on ZonedDateTime.

An added possible advantage of the change is that we can change the name of the new method to include "TimeZone" in the method name (which is important context for users because it will now be on ZDT instead of on a TimeZone class) without having to also include Next and Previous in the method name which would balloon its length.

The champions did want to make clear that we strongly agree with the comments from @littledan above: turning separate methods into a parameter on merged method is a step backwards in usability. We're OK to it here because these are advanced-user-only methods, but in general we will not be doing this kind of parameterized multiplexing because it degrades discoverability without actually removing complexity from the API for implementers or userland developers.

I'll follow up this comment to kick off bikeshedding the new method name and the parameter shape and values.

[justingrant]

I'll kick off the bikeshedding for this merged method. Feel free to add more ideas or add your feedback! FYI @sffc @ptomato @gibson042 @gilmoreorless @arshaw @ctcpip for your thoughts.

Method names

• getTimeZoneTransition(arg) - This is my preference...
• getZoneTransition(arg) - ...but I don't mind this either
• getTransition(arg) - I like this because it's shorter, but I worry that users might not know what it is because there's no "time zone" context in the name
• timeZoneTransition(arg) - I'd worry that some users might think that this is a boolean, e.g. "is this a time zone transition?", but putting get in front makes the type a bit clearer.

Parameter shapes

• string enumeration positional parameter - this is my preference
• options object - IMO, seems more complicated than needed here. If we ever wanted to add more choices (e.g. first transition) then we can always add more values to the string enumeration.
• number (e.g. -1 => backwards, 1 => forwards) - This introduces more failure cases (e.g. 0, -0, NaN) and is less IDE-discoverable than a string enumeration, so I don't like this one.

String enumeration parameter values

• "next" | "previous" - I slightly prefer this because it's closer to what we're already using, and it aligns with use elsewhere in the web platform, e.g. previousSibling
• "earlier" | "later" - this is nice because we use the same strings in disambiguation, but the semantics of the disambiguation option are different, so I worry a bit using the same strings to mean two different things in different methods.
• "forward" | "backward"

[sffc]

TC39 has previously recommended using the .get prefix for functions. tc39/proposal-intl-locale-info#67

I think my preference would be

zdt.getTimeZoneTransition({ direction: "..." })

I think I prefer "next" | "previous"

I prefer an options bag because this option configures the behavior, and the argument is not described in the function name like it is in the handful of positional argument functions we have like .withTimeZone() which clearly suggests that the first argument is a time zone. It's also imagineable that additional options could be desired in the future, like { skip: 3 } or { timeOfYear: "springForward" } or something like that.

Question: are these inclusive-next or exclusive-next? What happens if you call them on a time zone transition itself?

zdt.getTimeZoneTransition({ direction: "next" }).getTimeZoneTransition({ direction: "next" })

If they are inclusive-next, you can always skip ahead by adding 1 nanosecond before calling getTimeZoneTransition()

[justingrant]

inclusive-next or exclusive-next? What happens if you call them on a time zone transition itself?

They're exclusive. it starts looking 1ns before or 1ns after the receiver.

[arshaw]

I think it's important to reference some sort of "movement" in the method name to avoid this misinterpretation:

if (!zdt.getTimeZoneTransition()) {
  console.log('A time zone transition does not occur at the given ZonedDateTime!')
}

// ^^^obviously wrong because the API returns the *next* transition,
// NOT whether a transitions happens AT the given ZonedDateTime

Out of all the "movement" words we could choose ("next" / "later" / "forward"), I like "next" the best because it's generic enough to be used for either direction.

The following is my preference:

// moving towards future (default)
zdt.getNextTransition()

// moving towards past
zdt.getNextTransition({ reverse: true })

[justingrant]

Meeting 2023-05-30:

We will merge these two methods into a new getTimeZoneTransition method on Temporal.ZonedDateTime.

It will accept a single required argument which must be either 'next' | 'previous' or { direction: 'next' | 'previous' }. This shape is consistent with other Temporal required options bags like Duration.p.total where a string enumeration argument can replace an options bag with the most important (or only, as in this case) option filled in.

[ptomato]

Done in #2914.