Proposal-temporal: Feedback issue for names of conversion methods

Created on 10 Jul 2020  Â·  23Comments  Â·  Source: tc39/proposal-temporal

This issue is a place to consolidate all of the feedback that people are giving about the names of the methods that convert between Temporal types. These are important to get right since we expect that they will be very often used, so it's worth a bit of bikeshedding here.

Note that there are two kinds of conversions: those that add information (e.g. YearMonth + day → Date) and those that drop information (e.g. DateTime – Time → Date). Note we are only talking about the first kind (adding information) since there are no serious proposals yet for renaming the second kind (but see #724).

Summary of considerations so far:

  1. toTargetType (toDateTime, toYearMonth, etc.) or getTargetType or similar

    • pros

    • goal-focused, API users likely to be thinking of what type they want

    • type to in an IDE and the autocomplete will show you what types you can get, aiding in learning the types

    • cons

    • great for information-dropping conversions, merely okay for information-adding conversions

    • arguments can be ambiguous if they are number literals (mitigated with YearMonth.toDateOnDay and MonthDay.toDateInYear)

  2. withAddedInformation (withYear, withDate, etc.) or atAddedInformation (atTime, atDay, etc.)

    • pros

    • very intuitive about what information is being combined and therefore what information is contained in the target type

    • the "at" naming specifically aligns with prior art in other languages and fits with possible methods like atStartOfDay

    • cons

    • doesn't make sense for information-dropping conversions so two styles of method names are needed in any case

    • hard to reason about the types involved if you're not already familiar with the Temporal types

  3. toTargetTypeWithAddedInformation (toDateWithYear, toDateTimeWithDate, etc.)

    • pros

    • all the advantages of options 1 and 2 combined, and none of the disadvantages

    • cons

    • long, hard-to-read method names

bikeshed ergonomics feedback

All 23 comments

@ptomato - this issue is only about naming for "adding information" conversions, right? If yes, then should probably clarify in issue title/description. If not, then I'm confused because option 2 doesn't apply to "removing information" cases.

Also, for naming of "remove information" conversions, AFAIK the only possible change would be #724 to turn them into read-only property getters. Are there any others? If not then should probably point interested folks over to #724. (I'm aware that this change is unlikely to happen for functional concerns like SES, RAM usage, etc. but coalescing that discussion into one issue would be good.)

Finally, you mention the toTargetWithSomething pattern above (e.g. toDateWithDay) as part of Option 1, but this feels like it could also be a separate Option 3 that combines Option 1 and Option 2 at the expense of brevity.

this issue is only about naming for "adding information" conversions, right?

That's not what I intended, but it does make sense to narrow the scope of the discussions, because there are no serious alternatives for dropping-information conversions other than toX / getX besides #724 which you mentioned, but I think that should be outside the scope of this. I'll edit the OP to reflect this.

Finally, you mention the toTargetWithSomething pattern above (e.g. toDateWithDay) as part of Option 1, but this feels like it could also be a separate Option 3 that combines Option 1 and Option 2 at the expense of brevity.

Good point, I'll add that as well.

I transfer my arguments for using -at or -with here as they are now scattered across multiple issues.

The toTargetType option (1) makes long functions for:

  • YearMonth.toDateOnDay
  • MonthDay.toDateInYear
  • YearMonth.toDateAtEndOfMonth (proposal)
  • Date.toDateTimeOnStartOfDay (proposal)

Secondly, it uses different prepositions "on day", "in year", "at end of month".

Using with- we have:

  • YearMonth.withDay
  • MonthDay.withYear
  • YearMonth.withDayAtEndOfMonth
  • Date.withTimeOnStartOfDay

Still using different prepositions at and on.

Using at- is even more concise:

  • YearMonth.atDay
  • MonthDay.atYear
  • YearMonth.atEndOfMonth
  • Date.atStartOfDay

at- also results into a single preposition, is the most concise, and is aligned with the Java Time lib and the libs derived from it.

My personal preference is still option 2, but I'm not opposed to keeping the option 1 that we have now. Option 3 seems less desirable since we have gotten a lot of feedback that the API is too much typing as it is already.

Might I propose an option 4? We could drop the additive methods altogether, leaving just static functions (e.g., Temporal.Date.from({ ...monthDay.getFields(), year })) and lossy to\/with* (or option 4b, dropping those as well).

  • pros

    • minimizes API concepts, especially in variant 4b (expressions always start with the desired type)

    • makes virtualizing Temporal an order of magnitude easier (no need to monkey-patch anything other than Temporal.now and Temporal[…].from), which is good for testing/SES/etc.

  • cons

    • more verbose than prototype methods, especially if fields are not own properties (cf. https://github.com/tc39/proposal-temporal/issues/720#issuecomment-681605084 )

I like option 4a, with the lossy functions prefixed with get.

It works nicely with calendar systems.

const monthDay = Temporal.MonthDay.from({ calendar: "japanese", month: 1, day: 1 });

// Status quo
monthDay.toDate(2);  // Error!  But why?
monthDay.toDate({ year: 2, era: "reiwa" });  // OK

// Option 4a
Temporal.Date.from({ ...monthDay.getFields(), year: 2 });  // Error!  I need more things in my property bag
Temporal.Date.from({ ...monthDay.getFields(), year: 2, era: "reiwa" });  // OK

I propose option 5: remove toDate methods from MonthDay and YearMonth but otherwise keep the status quo of toXxx for all other conversions. This is my preferred option.

Most of the concerns noted above seem to stem from those two types. Those two types are almost certain to be the least-used Temporal types, so IMHO having their date conversions be different from others may not be a big deal if most developers will rarely or never use those types _(EDIT: removed that last presumptious statement)_.

I don't support option 4, for two reasons:

A) 4a/4b doesn't work for Absolute which has no fields but does have toDateTime and (soon) toLocalDateTime conversion methods. In theory Absolute.prototype.toLocalDateTime could be replaced via a call to the LDT constructor but that seems really ugly. I don't think there's any alternative to Absolute.prototype.toDateTime` though. So any 4a/4b solution will be inconsistent between additive conversions on Absolute vs. other types. If we must have inconsistency, I'd rather optimize for consistency between frequently used types: Date, Time, DateTime, LocalDateTime, and Absolute. If MonthDay/YearMonth are different, IMHO that's better because those types will be used less than other Temporal types regardless.

B) 4a/4b would be degrading the usability of toLocalDateTime methods. The current plan is to rely on toLocalDateTime methods for many use cases. Here's a few:

// Same local time in different time zone
ldt.toDateTime().toLocalDateTime(otherTz);

// parse a bracketless ISO string into a LocalDateTime with an offset time zone
Temporal.Absolute.from(isoString).toLocalDateTime(Temporal.TimeZone.from(isoString));

// date picker result in current time zone
datePicker.value.toLocalDateTime(Temporal.now.timeZone());

Here's the same use cases in 4a.

// Same local time in different time zone
// If you forget to set the offset to `undefined`, you'll get an exception due to a zone vs. offset conflict
// or (in rare cases like adjacent time zones during DST repeated hours) you could get the "wrong" repeated time.  
Temporal.LocalDateTime.from({...ldt.toDateTime().getFields(), timeZone: otherTz});

// parse a bracketless ISO string into a LocalDateTime with an offset time zone
new LocalDateTime(Temporal.Absolute.from(isoString).getEpochNanoseconds(), Temporal.TimeZone.from(isoString));

// date picker result in current time zone
Temporal.LocalDateTime.from({...datePicker.value.getFields(), timeZone: Temporal.now.timeZone()});

The current plan is to put these methods on Date, Time, DateTime, TimeZone, and Absolute. See #700 for more details in the "Conversions and Integration With the Rest of Temporal" section. (There's no planned toLocalDateTime method on MonthDay and YearMonth, for the same reasons as other comments in this issue: their parameters would be confusing.)

I actually like your 4a examples except "Same local time in different time zone", which should be

Temporal.LocalDateTime.from({...ldt.getDateTime().getFields(), timeZone: otherTz});

And I'd be willing to special-case Absolute, either with projection methods or with other-class static functions like fromEpochNanoseconds.

I actually like your 4a examples except "Same local time in different time zone", which should be

Temporal.LocalDateTime.from({...ldt.getDateTime().getFields(), timeZone: otherTz});

AFAIK, this is incorrect. timeZoneOffsetNanoseconds is a field, so the code above will throw if the old offset conflicts with the new time zone.

Temporal.DateTime instances don't carry time zone, and therefore don't have timeZoneOffsetNanoseconds.

Temporal.DateTime instances don't carry time zone, and therefore don't have timeZoneOffsetNanoseconds.

Oops, good catch. Edited. Also thanks for fixing my other typos! I'm running on fumes after little sleep. ;-)

@justingrant Can you explain more about why you prefer removing toDate from YearMonth and MonthDay above the status quo?

@justingrant Can you explain more about why you prefer removing toDate from YearMonth and MonthDay above the status quo?

Per our principles from #874, my main concern is that we don't make other types worse because the pattern used elsewhere doesn't work here. That's my main concern: I don't want to degrade usability of more-used types just because they don't work well on these types.

For these types, removing is OK with me. So is using a different naming pattern like toXXXonYYY.

It's also OK if we decide on a different pattern for the other types too-- I just don't want to use these less-commonly-used types as the reason to make those changes.

Ignoring YearMonth/MonthDay issues, I see three main advantages of using methods over from(...xxx.getFields()):

  • IDE discoverability. Just type "to" and you can get the right code using just arrow keys and Enter. If you don't know the from() trick, it's essentially impossible to figure it out without reading the docs. It's made worse by the fact that from() is used for both strings and objects, so if you first see from used with a string while reading code, you might not realize that you can also use it for objects too.

  • Chaining. It's clearer to read and code that moves from left to right to match the data flow, but code using from has to be written from right-to-left or (if there's multiple spreads) "inside out". For example, here's a medium-complexity use case: "For a meeting on a given date that starts at noon and has a given length , what time does it end in London?"

// chained
date.toDateTime('12:00').plus(duration).toLocalDateTime('Europe/London').toTime();

// rely on from()
Temporal.Time.from(
  Temporal.LocalDateTime.from({
    ...Temporal.DateTime.from({
      ...date.getFields(),
      ...Temporal.Time.from('12:00').getFields()
    })
      .plus(duration)
      .getFields(),
    timeZone: 'Europe/London'
  })
);

I know there's a cleaner way to implement the from variant (e.g. could use with in some spots) but my point is that from leads to code where the data flow is really hard to follow.

FWIW, I was completely unable to indent the from version manually-- I had to use Prettier. And honestly I'm not even 100% sure the second variant is equivalent to the first because it's so hard to follow the second one.

Between the indenting, the back-n-forth data flow, and the lack of IDE autocomplete the second variant took me at least 5x more time to write, and I'd expect a similar disparity when you read it.

  • Brevity. The other two above are higher priority IMHO, but we know that a decent percentage of developers really hate extra typing.

Okay. I'm back onboard with the conversion methods.

I like Option 1 (toTargetType) better than Options 2 (withAddedInformation) or 3 (toTargetTypeWithAddedInformation) because the added information is not always consistent. For example, going from a MonthDay to a Date requires a year most of the time, but it also requires an era if you are in the Japanese calendar. Therefore, toDateWithYear is misleading.

toDateInYear({ era: 'reiwa', year: 2 }) reads pretty naturally as "convert to a date in year 2 of the Reiwa era" to me?

date.toDateTime('12:00').plus(duration).toLocalDateTime('Europe/London').toTime()

I agree with your thoughts on data flow, and even though the pipeline operator would address most of them, it wouldn't be able to support the convenience of using '12:00' rather than Temporal.Time.from('12:00')—color me convinced on that point, even though I still have reservations about the impact of data-additive methods on virtualization¹. However, I do find it confusing that the expression uses .to… methods twice to _add_ data and once to _extract_ it. Should Temporal adopt a naming pattern that exposes the difference between methods that return a richer type vs. a simpler type vs. the same type (e.g., respectively dateTime.toLocalDateTime(timeZone) vs. dateTime.getTime() vs. dateTime.with(fields))?


¹ For example, Temporal.TimeZone.from = function(timeZone){…} currently intercepts time zone resolution, but Temporal.TimeZone = class{ static from(item){…} … } does not, because the specification of ToTemporalTimeZone defers to TimeZoneFrom, which uses a privileged reference to the built-in %Temporal.TimeZone%.

I agree with your thoughts on data flow, and even though the pipeline operator would address most of them

pipeline-operators are unnecessary in dynamically-typed javascript.
in @justingrant's example, baton-passing a dynamically-typed temp-variable is more readable/maintainable:

async function foo (date) {
    // use dynamically-typed temp-variable as baton
    let result;
    result = date;
    // convert date to datetime
    result = Temporal.DateTime.from({
        ...result.getFields(),
        ...Temporal.Time.from('12:00').getFields()
    });
    // add duration
    result = result.plus(duration);
    // convert datetime to localdatetime
    result = Temporal.LocalDateTime.from({
        ...result.getFields(),
        timeZone: 'Europe/London'
    });
    // convert localdatetime to time
    result = Temporal.Time.from(result);
    // async-fetch result using time
    result = await fetch("https://www.example.com?" + JSON.stringify(result));
    result = await result.json();
    return result;
}

However, I do find it confusing that the expression uses .to… methods twice to _add_ data and once to _extract_ it. Should Temporal adopt a naming pattern that exposes the difference between methods that return a richer type vs. a simpler type vs. the same type (e.g., respectively dateTime.toLocalDateTime(timeZone) vs. dateTime.getTime() vs. dateTime.with(fields))?

IMHO, it's a significant additional cognitive load to ask developers to think about about whether a conversion is additive or reductive, especially in cases like Absolute<->DateTime conversions which are not obviously in either camp. Also, toString doesn't fit the pattern. Even ignoring border cases, Temporal's "conceptual learning budget" is already sky-high. Adding more concepts for conversion categories isn't where I'd want to spend scarce learning budget for new users.

I'd also worry that having a bifurcated (or trifucrcated!) naming convention makes it riskier that any future extensions to Temporal (e.g. new types, new interop cases, etc.) would produce more methods on the border between those categories.

Finally, from an IDE or browser-devtools discoverability standpoint, the toXxx pattern is ideal. Every JS developer knows toString, and when they start typing it they'll be shown a list of types that this instance can be converted into, with inline documentation of each conversion accessible via the down arrow key. For users who have never written any Temporal code before, this will help users discover new Temporal types and learn how Temporal types relate to each other. Splitting conversions into two categories makes discovery and learning harder.

For the reasons above, I strongly support ("4" on our 1-5 scale) a single pattern. I'd prefer that pattern to be toXxx to align with toString for consistency and discoverability (and for one less character of typing vs. getXxx), but I'm only a 2 in 5 on that opinion.

I could probably be convinced to ditch toXxx for reductive conversions in favor of property getters (e.g. Date.prototype.yearMonth) if the problems noted in #724 could be resolved, but currently I prefer the status quo.

Does that imply also renaming with (e.g. getting the first day of the next month like Temporal.now.date().toDate({ day: 1 }).plus({ months: 1 })), or does "single pattern" actually encompass two patterns (one for field replacement and a different one for conversion)?

Does that imply also renaming with (e.g. getting the first day of the next month like Temporal.now.date().toDate({ day: 1 }).plus({ months: 1 })), or does "single pattern" actually encompass two patterns (one for field replacement and a different one for conversion)?

Hmm, interesting proposal. My first impression is that "immutating" this feels like a different enough operation from type conversion that using different names is probably desirable. But for the same reasons I listed in comments above, I'd support renaming with if combining the patterns didn't hurt discoverability of what we currently call with.

For example, combining the patterns of "_turn this into another type by removing data_" and "_turn this into another type by adding data_" results in "_turn this into another type_" which reduces Temporal's complexity budget (via one less concept) without IMHO the resulting pattern being any harder to discover.

Could one do the same thing when combining with? Is there an even simpler description than "_turn this into another type_" for the resulting combined pattern? If not, then merging with into the toXxx pattern would be less compelling.

To keep this issue focused on method names only, I deleted my last comment and moved its content into #889.

TL;DR of #889 - Some of the concerns in this bikeshed thread aren't about method names. Instead, they're about confusing parameters to conversion methods. Can we come up with a better pattern for these methods parameters, which might take some pressure off of the names?

Due to the adoption of the proposal in #889, and general support of toFoo as the pattern for conversion method names, there's nothing else to do for this issue. Closing.

Was this page helpful?
0 / 5 - 0 ratings