moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.4k stars 729 forks source link

Retrieving a locale string with `second: '2-digit'` returns '0' instead of '00' #1418

Closed nathanlesage closed 1 year ago

nathanlesage commented 1 year ago

Describe the bug

When requesting the locale string for a '2-digit' second, the library returns '0' instead of '00'.

To Reproduce

import { DateTime } from 'luxon'
const now = DateTime.now()

// For reference, this also happens when instantiating from a fixed string:
const now = DateTime.fromISO('2017-12-26T21:12:00.000+01:00')

// Prints "0", as expected
console.log(now.toLocaleString({ second: 'numeric' })
// Also prints "0", instead of the expected "00"
console.log(now.toLocaleString({ second: '2-digit' }))

Actual vs Expected behavior

When requesting a "2-digit" version of the second, it should've printed "00" instead of "0"

Desktop (please complete the following information):

Additional context

I just discovered a function in one of my projects that acts sort of as a strptime drop-in, but that was still on moment.js. I decided to move that to luxon as well, which is when I discovered this behavior.

diesieben07 commented 1 year ago

To start off, let me clarify that Luxon's toLocaleString is mostly a very thin wrapper around Intl.DateTimeFormat, so it depends on the underlying platform. What you observe is a side-effect of the fact that the output of toLocaleString is not 100% guaranteed, especially for "unusual" parts combinations. Not all combinations of properties for toLocaleString are supported by all platforms. You can find the combinations which are definitely supported in the Intl.DateTimeFormat documentation (see "The following properties describe the date-time components to use in formatted output, and their desired representations. Implementations are required to support at least the following subsets" and following).

Specifying just second is not in the list of supported property combinations, and it looks like V8 doesn't support 2-digit in this case (which is a valid implementation of the Intl spec). If you specify hours, minutes and seconds, you have a supported combination of properties and thus you're "within the spec" and V8 behaves properly.

What you can do:

I hope this helps.

nathanlesage commented 1 year ago

The toFormat function has worked perfectly well, and it is indeed even more what I personally was looking for. So thank you for your suggestion!

The reason why I did not use toFormat initially was that the API docs state, in bold letters, "You may not want this". I always try to follow the recommendations of library's authors, and therefore refrained from using this function.

I think simply un-bolding this "You may not want this" would suffice to make users less hesitant of using the formatter, but I cannot speak for the rest of Luxon's users, so I'm trusting your judgment here.

Nevertheless, it may make sense to warn users about the potentiality that even while typing 2-digit, one could end up with only one Maybe a warning in the toLocaleScring API documentation that, depending on the implementation, 2-digit may not mean two digits, could be in order?

But that's OOS for this issue, which I'm closing now :)

diesieben07 commented 1 year ago

Fair point, the docs can always use improvement. I think adding the note about 2-digit being a suggestion (at least in many cases) is a good idea.