search

js-temporal / proposal-temporal-v2

Future additions to Temporal
MIT License
24 stars 1 forks source link

Convert Instant/ZonedDateTime -> Date #31

Open pauldraper opened 9 months ago

pauldraper commented 9 months ago

Add Date.fromTemporalInstant and Date.fromTemporalZonedDateTime function to easily create Dates from Temporal values.

Advantages:

Converting to Date currently requires:

new Date(instant.epochMilliseconds)
new Date(zonedDateTime.epochMilliseconds)

This is awkward, and requires remembering the unit for Dates. (seconds? millis? nanos?) For adoption, it's important that converting to/from Dates is easy and rebost.

This proposal offers convenience functions:

Date.fromTemporalInstant(instant)
Date.fromTemporalZonedDateTime(zonedDateTime)

Concerns:

See:

Converting from ZonedDateTime and Date suggests that Date has time zone (not just offset)

Downcasting is common. No one would think that ZonedDateTIme -> Instant suggests that Instant has time zone.

Usage of Date should be discouraged

This sentiment vastly underestimates the stickiness of current Date usage and the necessity of easy conversion between the two.

new Date(instant.epochMilliseconds) is easy enough

With all due respect, not at all. For example MDN list the following constructor calls for Date:

new Date()
new Date(value)
new Date(dateString)
new Date(dateObject)

new Date(year, monthIndex)
new Date(year, monthIndex, day)
new Date(year, monthIndex, day, hours)
new Date(year, monthIndex, day, hours, minutes)
new Date(year, monthIndex, day, hours, minutes, seconds)
new Date(year, monthIndex, day, hours, minutes, seconds, milliseconds)

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/Date

I'm supposed to know that I should pass one arg, and that arg is milliseconds? Come on, man.

Prior art:

N/A

Constraints / corner cases:

N/A

justingrant commented 9 months ago

Thanks for this suggestion. Date.fromTemporalInstant(instant) seems like a reasonable addition to the API, and it's consistent with Date.toTemporalInstant(instant) which performs the opposite conversion in the V1 of Temporal.

I'd be concerned about Date.fromTemporalZonedDateTime(zonedDateTime) however, because while gathering community usage experience of Date to inform the design of the Temporal API, we found it to be a surprisingly common mistake made by novice JS developers to assume that Date contains a time zone. Example: https://stackoverflow.com/questions/15141762/how-to-initialize-a-javascript-date-to-a-particular-time-zone

For developers who think Date has a time zone, it'd be logical for them to assume that Date.fromTemporalZonedDateTime(zonedDateTime) carries the time zone from Temporal.ZonedDateTime over to the Date instance. If the Temporal.ZonedDateTime instance had the user's time zone, it might take time to discover the bug because Date appears to have the same time zone.

For this reason, if we do extend Date<=>Temporal.* conversions, I'd recommend that we limit to Temporal.Instant only, in order to help those novice developers learn that converting from Temporal.ZonedDateTime loses the time zone along the way.

pauldraper commented 9 months ago

I agree with your assertion that many developers assume that Date has a timezone.

However:

  1. Whatever % of developers believe that today, I'm skeptical that the existence of Date.fromTemporalZonedDateTime() would increase it.
  2. The purpose of these functions is to interface with legacy Date code. If code relied on Date having a timezone, it will remain just as broken as it was before.

That said, most of the value is achieved by having Date.fromTemporalInstant().